Opened on 01/04/2018 at 02:19:44 PM
Closed on 08/30/2019 at 04:57:49 PM
#6230 closed change (rejected)
Alter resolve path of info module generated by buildtools
| Reported by: | kzar | Assignee: | |
|---|---|---|---|
| Priority: | Unknown | Milestone: | |
| Module: | Automation | Keywords: | goodfirstbugs |
| Cc: | tlucas, sebastian, hfiguiere, sergz, erikvold, geo | Blocked By: | |
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | no | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description (last modified by kzar)
Background
Recently we switched to using Webpack for our module system when building adblockpluschrome. We're now able to adjust the require paths to be relative and that includes the requires in adblockpluscore (#5762).
With how we've configured Webpack currently the special info module which is generated at build time is considered to be located in adblockpluschrome/buildtools/info.js when building the extension. This causes us some problems (see this discussion) since libadblockplus stores the file in libadblockplus/lib/info.js. For require("../../lib/info") to work for both libadblockplus and adblockpluchrome we need to adjust our Webpack configuration.
What to change
Adjust webpack_runner.js so that the info alias points to the parent directory, e.g.
info$: path.join(__dirname, "info.js"),
becomes
info$: path.join(extension_path, "lib", "info.js"),
Remove the stub buildtools/info.js file.
Integration notes
- Create an stub file in adblockpluschrome/lib/info.js.
- Ensure any relative requires for the info module are updated with the correct path.
Attachments (0)
Change History (7)
comment:1 Changed on 01/04/2018 at 02:37:32 PM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:3 Changed on 01/05/2018 at 03:34:56 PM by kzar
- Blocking 5762 removed
comment:4 Changed on 04/11/2018 at 03:35:53 PM by kzar
- Keywords goodfirstbugs added
- Status changed from reviewing to reopened
comment:5 Changed on 04/11/2018 at 03:36:04 PM by kzar
- Owner kzar deleted
comment:6 Changed on 11/15/2018 at 03:40:19 AM by erikvold
- Cc erikvold added
comment:7 Changed on 08/30/2019 at 04:57:49 PM by sebastian
- Cc geo added
- Resolution set to rejected
- Status changed from reopened to closed
Maybe this is something that we can address in the new build suite, Geo is working on. But as our legacy buildtools is on the way out, we won't address it there for the time being.
Since there was disagreement about calling require("../../lib/info") from adblockpluscore this change is no longer blocking #5762. Nevertheless it seems to me a sensible change to make so I'll leave this issue open.