Closed Bug 1190024 Opened 9 years ago Closed 9 years ago

Place JS files at same path as source dir

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: jryans, Unassigned)

References

(Blocks 3 open bugs)

Details

As part of cleaning up DevTools file loading, I would like to stop explicitly stating the module path in moz.build. One of our goals is for require() paths that import other modules to match the source location on disk. The require path is driven by the module path that moz.build installs the JS module to. Since they are currently explicit, nothing ensures that the paths match the location of source on disk. If a directory is moved, or someone copies the moz.build contents from some other place in the tree, we can end up with incorrect paths. So, for the tree: /devtools/client ├── example.js └── moz.build I want /devtools/client/moz.build to install example.js to "modules/devtools/client/example.js" based solely on the current path in the source tree. Perhaps a new moz.build field separate from EXTRA_JS_MODULES is needed to indicate that this should be done.
Nathan, it looks like you've investigated some things related to EXTRA_JS_MODULES. Do you know if there is an existing way in moz.build to achieve the concept above, or would some new field need to be added?
Flags: needinfo?(nfroyd)
You actually shouldn't need anything new to do this. You can create a template that does what you want. For example (untested, but you get the idea): @template def DevToolsModules(*modules): base = EXTRA_JS_MODULES for dir in SRCDIR.split('/'): base = base[dir] base += modules Then you can use, in /devtools/client/moz.build: DevToolsModules( 'example.js', 'foo.js', ) And that should install example.js and foo.js in dist/bin/modules/devtools/client
Flags: needinfo?(nfroyd)
(In reply to Mike Hommey [:glandium] from comment #2) > You actually shouldn't need anything new to do this. You can create a > template that does what you want. > > For example (untested, but you get the idea): > > @template > def DevToolsModules(*modules): > base = EXTRA_JS_MODULES > for dir in SRCDIR.split('/'): > base = base[dir] > base += modules Almost worked... just needs RELATIVEDIR instead of SRCDIR, and [m for m in modules] instead of modules. BTW, while I think about this: please don't forget that your browser/ specific files should go in resource://app/modules, not resource://gre/modules
As always, Mike knows more than I do. :)
Ah, great! I'll give this a try soon! Thanks Mike. Will update bug status based on my results.
Mike, this works quite well in nearly every case. There's one issue I did find, which is with two calls of DevToolsModules() in the same file, which may happen if there are some conditionally included files, etc. If there are two invocations of DevToolsModules() in the same file, I get the error: 0:08.27 ============================== 0:08.27 ERROR PROCESSING MOZBUILD FILE 0:08.27 ============================== 0:08.27 0:08.27 The error occurred while processing the following file: 0:08.27 0:08.27 /Users/jryans/projects/mozilla/gecko/devtools/client/moz.build 0:08.27 0:08.27 The error was triggered on line 47 of this file: 0:08.27 0:08.27 DevToolsModules('main.js') 0:08.27 0:08.27 The underlying problem is an attempt to reassign a reserved UPPERCASE variable. 0:08.27 0:08.27 The reassigned variable causing the error is: 0:08.27 0:08.27 EXTRA_JS_MODULES 0:08.27 0:08.27 Maybe you meant "+=" instead of "="? I tried a few variations of the template, but couldn't find a good way to work around this. Any ideas?
Flags: needinfo?(mh+mozilla)
There are two things that prevent that from working: - When merging the evaluation of a template back in the parent moz.build context, we handle different types in different ways: http://hg.mozilla.org/mozilla-central/file/095988abdc56/python/mozbuild/mozbuild/frontend/reader.py#l405 but EXTRA_JS_MODULES is a HierarchicalStringList, and that's the only list type in mozbuild.util that doesn't inherit from list, so isinstance(value, list) doesn't match it. - Even after replacing with isinstance(value, (list, HierarchicalStringList)), that still doesn't work, because HierarchicalStringList doesn't support +='ing another HierarchicalStringList. It only accepts lists: http://hg.mozilla.org/mozilla-central/file/095988abdc56/python/mozbuild/mozbuild/util.py#l576 So, either you fix that, or you prepare a list of files beforehand depending on your conditions, and you call DevToolsModules(*that_list)
Flags: needinfo?(mh+mozilla)
Thanks for the explanation Mike, I was guessing it might changes to mozbuild itself to support, as you've now made clear. Since there's only one case of this so far, I'll just prepare a list and only invoke once. I'll also file follow up bug to potentially make a real fix later. Marking this one WORKSFORME, since Mike's advice works great and there were no actual code changes here.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.