Many of the paths in Loader.jsm are redundant and unnecessary. Let's reduce them down where possible! Also, let's add giant warnings to avoid adding more lines. :D

I also wouldn't mind moving some of the js modules that are in toolkit/devtools into the new toolkit/devtools/shared folder just to clean up that directory a bit

(In reply to Brian Grinstead [:bgrins] from comment #1) > I also wouldn't mind moving some of the js modules that are in > toolkit/devtools into the new toolkit/devtools/shared folder just to clean > up that directory a bit Yeah, sounds like a great idea! The one thing this work needs to be careful of though is DevTools addons in the wild. It's likely this work would change the require paths need to get various things. We can fix in-tree, but not add-ons. So, either there needs to be some kind of legacy mapping, or we make the add-ons use different paths per Firefox version. Not sure yet how bad the damage would be.

Ouch! gcli is doing some complex file <> module mapping: http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/gcli/moz.build#7 Both files from toolkit/devtools/gcli/commands and toolkit/devtools/gcli/source/lib/gcli/commands get merged into require("gcli/commands/*") :-/ This is fine with a build system, but won't work with `tools srcdir` feature...

Joe, may we split them (back?) to two distinct folders and require paths? http://mxr.mozilla.org/mozilla-central/search?string=gcli/commands I can easily rewrite all these occurences to require the files like this: toolkit/devtools/gcli/source/lib/gcli/commands => require(devtools/toolkit/gcli/commands) toolkit/devtools/gcli/source/lib/gcli/commands => require(gcli/commands)

Do you mean: toolkit/devtools/gcli/commands => require(devtools/toolkit/gcli/commands) toolkit/devtools/gcli/source/lib/gcli/commands => require(gcli/commands) ?

Yes, I messed up with my copy paste. Would that work for you?

(In reply to Alexandre Poirot [:ochameau] from comment #6) > Yes, I messed up with my copy paste. > Would that work for you? Yes, sounds good.

Simple fix, we were passing path splits to fileURI instead of join... https://treeherder.mozilla.org/#/jobs?repo=try&revision=37beeca647cf

When using gcli and local source mapping, this command keep throwing in the console, that's because there is a magic build-time mapping that moves resize-commands from responsivedesign folder to a top level folder. Like bug 1188417, we shouldn't do such magic build system mapping.

Comment on attachment 8641100 [details] [diff] [review] Fix broken module paths when mapping to source directory Review of attachment 8641100 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, but please move this to a new bug that blocks this one. I'd like to keep this bug focused on the step of removing path entries, which may depend on or be done as part of the file move (bug 912121).

Comment on attachment 8641102 [details] [diff] [review] Fix resize-commands module path when mapping to local sources Review of attachment 8641102 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, but please move this to a new bug that blocks this one. I'd like to keep this bug focused on the step of removing path entries, which may depend on or be done as part of the file move (bug 912121).

To clarify, the "path entries" I mean here are the require path to file / resource mappings in the Loader.jsm file itself.

Note that I tried to get rid of the magic require(source-map) path, but that raised some issues as stated in bug 935366 comment 12 and 15. We may have to keep some of these shortcuts.

(In reply to Alexandre Poirot [:ochameau] from comment #13) > Note that I tried to get rid of the magic require(source-map) path, > but that raised some issues as stated in bug 935366 comment 12 and 15. > We may have to keep some of these shortcuts. Yeah, we'll see. I am still hopeful most can go away, but we may need a "legacy" mapping of them, especially for add-ons. I'm planning to test some add-ons to see how bad it is once I've done the move.

Assigning this bug for now to acknowledge I intend to do this work, maybe as part of bug 912121. It would be sad to duplicate efforts when rewriting the whole tree is involved!

I no longer intend to make a change here. We have simplified the list somewhat already, but there may be a few more cleanups we could make.

What about just getting rid of the srcdir loader now that we have the addon?

(In reply to Alexandre Poirot [:ochameau] from comment #17) > What about just getting rid of the srcdir loader now that we have the addon? Yes, I think we do that. But, it seems like a separate bug from this. This is about removing duplicate paths, even in the "regular" loader.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #18) > (In reply to Alexandre Poirot [:ochameau] from comment #17) > > What about just getting rid of the srcdir loader now that we have the addon? > > Yes, I think we do that. But, it seems like a separate bug from this. This > is about removing duplicate paths, even in the "regular" loader. I think we *should* do that.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #18) > (In reply to Alexandre Poirot [:ochameau] from comment #17) > > What about just getting rid of the srcdir loader now that we have the addon? > > Yes, I think we do that. But, it seems like a separate bug from this. This > is about removing duplicate paths, even in the "regular" loader. Hum. Do you want to converge to have only just "devtools" mapping? (and the magic mapping for promise and xpcshell) Because I don't see any explicit duplicate entry now: http://mxr.mozilla.org/mozilla-central/source/devtools/shared/Loader.jsm#91 I opened bug 1250430 for the SrcDir removal

We still have acorn, which use a magic layout that is different between local and runtime, otherwise, we just have: - "" for SDK modules, - devtools to resource://devtools - some compatibility/handy shortcuts for sourcemap and gcli (and ideally also acorn). - shortcuts for promise jsm module. - something for xpcshell tests Ryan, do you thing we can close this bug if we fix acorn mapping?

(In reply to Alexandre Poirot [:ochameau] from comment #21) > We still have acorn, which use a magic layout that is different between > local and runtime, > otherwise, we just have: > - "" for SDK modules, > - devtools to resource://devtools > - some compatibility/handy shortcuts for sourcemap and gcli (and ideally > also acorn). > - shortcuts for promise jsm module. > - something for xpcshell tests > > Ryan, do you thing we can close this bug if we fix acorn mapping? I would like to get away from any random packages like sourcemap and gcli needing entries as well, but this may require more intelligence about how we handle packages. I mention them more explicitly in bug 1201710 which this depends on. At the same time, I don't think this is a big issue at this time, so I'd say keep this open for now, but there are probably other problems worth focusing on instead.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #22) > ... > I would like to get away from any random packages like sourcemap and gcli > needing entries as well For what it's worth all GCLI require statements are relative (e.g. [1]). So it's possible that if we change references to GCLI from outside /devtools/shared/gcli/source/lib (e.g. [2]) to point to the full path then that's all we need to do. It might even be as simple as s/require\("gcli/require("\/devtools\/shared\/gcli\/source\/lib/g That might be a bit of a mouthful for a require path, but probably better a require-path-mouthful than the mess in the loader. [1]: https://dxr.mozilla.org/mozilla-central/source/devtools/shared/gcli/source/lib/gcli/cli.js#19 [2]: https://dxr.mozilla.org/mozilla-central/source/devtools/client/shared/developer-toolbar.js#30