Closed
Bug 1160361
Opened 10 years ago
Closed 10 years ago
GCLI actor fails to getRequisition for new B2G servers
Categories
(DevTools Graveyard :: Graphic Commandline and Toolbar, defect)
DevTools Graveyard
Graphic Commandline and Toolbar
Tracking
(firefox40 fixed)
RESOLVED
FIXED
Firefox 40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: jryans, Assigned: jryans)
References
Details
(Keywords: regression, Whiteboard: [polish-backlog])
Attachments
(8 files, 1 obsolete file)
(deleted),
text/x-review-board-request
|
bgrins
:
review+
jwalker
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
bgrins
:
review+
jwalker
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
bgrins
:
review+
jwalker
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
bgrins
:
review+
jwalker
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
bgrins
:
review+
jwalker
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
bgrins
:
review+
jwalker
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
bgrins
:
review+
jwalker
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
bgrins
:
review+
jwalker
:
review+
|
Details |
GCLI now seems to assume files from /browser are available on servers, but that is not true for non-Firefox.
For example, a recent B2G nightly simulator build gives:
"Protocol error (unknownError): Module `devtools/commandline/commands-index` is not found at resource:///modules/devtools/commandline/commands-index.js" protocol.js:20
"Message: Module `devtools/commandline/commands-index` is not found at resource:///modules/devtools/commandline/commands-index.js
Stack:
GcliActor<._getRequisition@resource://gre/modules/devtools/server/actors/gcli.js:12:209
GcliActor<.specs<@resource://gre/modules/devtools/server/actors/gcli.js:2:734
actorProto/</handler@resource://gre/modules/devtools/server/protocol.js:74:9
DSC_onPacket@resource://gre/modules/devtools/server/main.js:97:215
ChildDebuggerTransport.prototype.receiveMessage@resource://gre/modules/devtools/transport/transport.js:38:337
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Comment 1•10 years ago
|
||
Hmmm, the solution to this could be via bug 1156682.
Alternatively we just could pull commands-index.js into toolkit, but I fear that's a bit like saying "Just make the toString() asynchronous", or "I'll just pull at the thread in this jumper to tidy it up".
Could a solution to bug 1160345 also provide a temporary solution to this problem?
Comment 2•10 years ago
|
||
Actually, on reflection I think injecting modules (comment 1) isn't the right solution.
I we should move commands-index.js from /browser/devtools/commandline to /toolkit/devtools/gcli and then wrap line 82 [1] in a try/catch (because that's where it all starts unravelling) and it might just work.
We may need to move command files into toolbox, and then hard-code them in commands-index, to make them available to b2g servers, but that can then be done one by one if needed.
[1]: https://dxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/commands-index.js#82
Updated•10 years ago
|
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Joe Walker [:jwalker] (overloaded - needinfo me or ping on irc) from comment #2)
> Actually, on reflection I think injecting modules (comment 1) isn't the
> right solution.
>
> I we should move commands-index.js from /browser/devtools/commandline to
> /toolkit/devtools/gcli and then wrap line 82 [1] in a try/catch (because
> that's where it all starts unravelling) and it might just work.
>
> We may need to move command files into toolbox, and then hard-code them in
> commands-index, to make them available to b2g servers, but that can then be
> done one by one if needed.
>
> [1]:
> https://dxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/
> commands-index.js#82
I've started down this path, so I'll see how far I can get.
Assignee | ||
Updated•10 years ago
|
Summary: Inspect element / command buttons no longer shown for new B2G servers → GCLI actor fails to getRequisition for new B2G servers
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
/r/8107 - Bug 1160361 - Move commands-index to the server. r=bgrins
/r/8109 - Bug 1160361 - Catch GCLI's load of definitions for non-Firefox. r=bgrins
/r/8111 - Bug 1160361 - Move gclicommands.properties to toolkit. r=bgrins
/r/8113 - Bug 1160361 - Move gcli.properties to toolkit. r=bgrins
/r/8115 - Bug 1160361 - Move GCLI prefs to all.js. r=bgrins
/r/8117 - Bug 1160361 - Load tilt commands from ToolboxButtons to avoid in non-Firefox. r=bgrins
/r/8119 - Bug 1160361 - Skip Telemetry calls for non-Firefox. r=bgrins
/r/8121 - Bug 1160361 - Abort tilt commands when remote. r=bgrins
Pull down these commits:
hg pull -r 7246c8e2a0e2fb91276650f0cfa1f831e9ff01cf https://reviewboard-hg.mozilla.org/gecko/
Attachment #8601083 -
Flags: review?(bgrinstead)
Comment 6•10 years ago
|
||
Comment on attachment 8601083 [details]
MozReview Request: bz://1160361/jryans
/r/8107 - Bug 1160361 - Move commands-index to the server. r=bgrins
/r/8109 - Bug 1160361 - Catch GCLI's load of definitions for non-Firefox. r=bgrins
/r/8111 - Bug 1160361 - Move gclicommands.properties to toolkit. r=bgrins
/r/8113 - Bug 1160361 - Move gcli.properties to toolkit. r=bgrins
/r/8115 - Bug 1160361 - Move GCLI prefs to all.js. r=bgrins
/r/8117 - Bug 1160361 - Load tilt commands from ToolboxButtons to avoid in non-Firefox. r=bgrins
/r/8119 - Bug 1160361 - Skip Telemetry calls for non-Firefox. r=bgrins
/r/8121 - Bug 1160361 - Abort tilt commands when remote. r=bgrins
Pull down these commits:
hg pull -r 7246c8e2a0e2fb91276650f0cfa1f831e9ff01cf https://reviewboard-hg.mozilla.org/gecko/
Attachment #8601083 -
Flags: review?(jwalker)
Comment 7•10 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #6)
> Comment on attachment 8601083 [details]
> MozReview Request: bz://1160361/jryans
>
> /r/8107 - Bug 1160361 - Move commands-index to the server. r=bgrins
> /r/8109 - Bug 1160361 - Catch GCLI's load of definitions for non-Firefox.
> r=bgrins
> /r/8111 - Bug 1160361 - Move gclicommands.properties to toolkit. r=bgrins
> /r/8113 - Bug 1160361 - Move gcli.properties to toolkit. r=bgrins
> /r/8115 - Bug 1160361 - Move GCLI prefs to all.js. r=bgrins
> /r/8117 - Bug 1160361 - Load tilt commands from ToolboxButtons to avoid in
> non-Firefox. r=bgrins
> /r/8119 - Bug 1160361 - Skip Telemetry calls for non-Firefox. r=bgrins
> /r/8121 - Bug 1160361 - Abort tilt commands when remote. r=bgrins
>
> Pull down these commits:
>
> hg pull -r 7246c8e2a0e2fb91276650f0cfa1f831e9ff01cf
> https://reviewboard-hg.mozilla.org/gecko/
Joe, I asked :gps to add you as a reviewer for one part to this (need to file a reviewboard bug to add this feature). I wanted you to take a look at changes to toolkit/devtools/gcli/source/* -> is it OK to land these in m-c first and then have them migrate back to the gcli repo?
Updated•10 years ago
|
Attachment #8601083 -
Flags: review?(bgrinstead) → review+
Comment 8•10 years ago
|
||
Comment on attachment 8601083 [details]
MozReview Request: bz://1160361/jryans
https://reviewboard.mozilla.org/r/8105/#review6889
The changes seem pretty straightforward. Before landing, I'd like to make sure Joe takes a look with regard to how we land the stuff inside the gcli source (and would welcome any feedback on the implementation from him).
::: toolkit/devtools/gcli/commands/index.js:1
(Diff revision 1)
> +/* This Source Code Form is subject to the terms of the Mozilla Public
Looks like this is a brand new file. Any way you could do a copy using hg / git so that we can maintain blame history? Same comment for all of the moved files in this commit queue.
Comment 9•10 years ago
|
||
https://reviewboard.mozilla.org/r/8105/#review6899
All looks good to me,
I'll handle syncing changes with the GCLI repo.
However, I think we need to take care when moving l10n files - translated copies of those files will need to be moved to keep up, I think. Maybe we should get feedback from :flod? Sorry to keep passing the f? !
Updated•10 years ago
|
Attachment #8601083 -
Flags: review?(jwalker) → review+
Comment 10•10 years ago
|
||
(In reply to Joe Walker [:jwalker] (overloaded - needinfo me or ping on irc) from comment #9)
> https://reviewboard.mozilla.org/r/8105/#review6899
>
> All looks good to me,
>
> I'll handle syncing changes with the GCLI repo.
>
> However, I think we need to take care when moving l10n files - translated
> copies of those files will need to be moved to keep up, I think. Maybe we
> should get feedback from :flod? Sorry to keep passing the f? !
Flags: needinfo?(francesco.lodolo)
Assignee | ||
Comment 11•10 years ago
|
||
Flod, for more context, I need to move 2 existing properties files from browser to toolkit.
Is there are a good way to handle this to keep translated files in sync?
Comment 12•10 years ago
|
||
If I read the patch right, gcli.properties and gclicommands.properties move from browser/devtools to toolkit/global/devtools
Are there any other changes besides the move?
The only thing we can do is to send out an email to localizers explaining that they can simply move the file in Mercurial, but that doesn't help people on external tools (hopefully translation memory will make things a little easier).
Side note: my only concern would be that these 2 files are huge. We talked in the past about splitting devtools from browser for localization, also to get a more realistic picture of the completion status of localized builds, but I have the feeling the more stuff moves to toolkit, the harder this would get.
Flags: needinfo?(francesco.lodolo)
Comment 13•10 years ago
|
||
Those were the only 2 files that I spotted.
Is there some variant of
for LOCALE in ....
do
hg clone https://hg.mozilla.org/l10n-central/${LOCALE}
cd ${LOCALE}
hg mv browser/devtools/.../gcli.properties toolkit/global/devtools/.../gcli.properties
hg mv browser/devtools/.../gclicommands.properties toolkit/global/devtools/.../gclicommands.properties
hg commit -m "Auto Move"
hg push
done
That would work?
Comment 14•10 years ago
|
||
(In reply to Francesco Lodolo [:flod] (UTC+2) from comment #12)
> Side note: my only concern would be that these 2 files are huge. We talked
> in the past about splitting devtools from browser for localization, also to
> get a more realistic picture of the completion status of localized builds,
> but I have the feeling the more stuff moves to toolkit, the harder this
> would get.
I'm unclear why it's harder from toolkit? Why's that?
Comment 15•10 years ago
|
||
(In reply to Joe Walker [:jwalker] (overloaded - needinfo me or ping on irc) from comment #13)
> for LOCALE in ....
> hg commit -m "Auto Move"
> hg push
> done
>
> That would work?
No, for two reasons. Localizers are responsible of their repositories, and that would simply not work for locales where external tools are committing to the repo.
(In reply to Joe Walker [:jwalker] (overloaded - needinfo me or ping on irc) from comment #14)
> I'm unclear why it's harder from toolkit? Why's that?
Those 2 files contain almost 500 strings. That moves 500 strings from Firefox (/browser) to any product we ship (Firefox, Firefox for Android, SeaMonkey, Thunderbird).
Example: a locale only shipping Fennec will grow by 500 strings in one commit.
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to Francesco Lodolo [:flod] (UTC+2) from comment #15)
> (In reply to Joe Walker [:jwalker] (overloaded - needinfo me or ping on irc)
> from comment #14)
> > I'm unclear why it's harder from toolkit? Why's that?
>
> Those 2 files contain almost 500 strings. That moves 500 strings from
> Firefox (/browser) to any product we ship (Firefox, Firefox for Android,
> SeaMonkey, Thunderbird).
>
> Example: a locale only shipping Fennec will grow by 500 strings in one
> commit.
It is more strings for all servers, but it seems required by the current design of GCLI, since it tells the client what commands exist, including their strings. I don't see an obvious way around this.
I'll proceed with landing (after fixing the patches to be file moves), and then notify localizers.
Assignee | ||
Comment 17•10 years ago
|
||
remote: https://hg.mozilla.org/integration/fx-team/rev/2a20b3052ed7
remote: https://hg.mozilla.org/integration/fx-team/rev/e92ab4bd581c
remote: https://hg.mozilla.org/integration/fx-team/rev/3f62eb1ee367
remote: https://hg.mozilla.org/integration/fx-team/rev/d7ca7e3d5aab
remote: https://hg.mozilla.org/integration/fx-team/rev/dc49f4668652
remote: https://hg.mozilla.org/integration/fx-team/rev/a737f0563c1f
remote: https://hg.mozilla.org/integration/fx-team/rev/5cdf7ebf5d6f
remote: https://hg.mozilla.org/integration/fx-team/rev/ba4ae6b6dcfc
Assignee | ||
Comment 18•10 years ago
|
||
Notice sent to dev-l10n:
https://groups.google.com/d/topic/mozilla.dev.l10n/OYsyDWSyZJg/discussion
Comment 19•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2a20b3052ed7
https://hg.mozilla.org/mozilla-central/rev/e92ab4bd581c
https://hg.mozilla.org/mozilla-central/rev/3f62eb1ee367
https://hg.mozilla.org/mozilla-central/rev/d7ca7e3d5aab
https://hg.mozilla.org/mozilla-central/rev/dc49f4668652
https://hg.mozilla.org/mozilla-central/rev/a737f0563c1f
https://hg.mozilla.org/mozilla-central/rev/5cdf7ebf5d6f
https://hg.mozilla.org/mozilla-central/rev/ba4ae6b6dcfc
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Assignee | ||
Comment 20•9 years ago
|
||
Attachment #8601083 -
Attachment is obsolete: true
Attachment #8620209 -
Flags: review+
Attachment #8620210 -
Flags: review+
Attachment #8620211 -
Flags: review+
Attachment #8620212 -
Flags: review+
Attachment #8620213 -
Flags: review+
Attachment #8620214 -
Flags: review+
Attachment #8620215 -
Flags: review+
Attachment #8620216 -
Flags: review+
Assignee | ||
Comment 21•9 years ago
|
||
Assignee | ||
Comment 22•9 years ago
|
||
Assignee | ||
Comment 23•9 years ago
|
||
Assignee | ||
Comment 24•9 years ago
|
||
Assignee | ||
Comment 25•9 years ago
|
||
Assignee | ||
Comment 26•9 years ago
|
||
Assignee | ||
Comment 27•9 years ago
|
||
Assignee | ||
Comment 28•9 years ago
|
||
Updated•9 years ago
|
Whiteboard: [devedition-40] → [polish-backlog]
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•6 years ago
|
Product: DevTools → DevTools Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•