1160361 - GCLI actor fails to getRequisition for new B2G servers

Status: RESOLVED FIXED

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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

Comment 1

[Joe Walker [:jwalker]]

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

[Joe Walker [:jwalker]]

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

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(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.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b12d609bce91

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: 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/

Comment 6

[Gregory Szorc [:gps]]

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/

Comment 7

[Brian Grinstead [:bgrins]]

(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?

Comment 8

[Brian Grinstead [:bgrins]]

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

[Joe Walker [:jwalker]]

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? !

Comment 10

[Brian Grinstead [:bgrins]]

(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? !

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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

[Francesco Lodolo [:flod]]

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.

Comment 13

[Joe Walker [:jwalker]]

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

[Joe Walker [:jwalker]]

(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

[Francesco Lodolo [:flod]]

(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.

Comment 16

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(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.

Comment 17

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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

Comment 18

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Notice sent to dev-l10n:

https://groups.google.com/d/topic/mozilla.dev.l10n/OYsyDWSyZJg/discussion

Comment 19

[Carsten Book [:Tomcat]]

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

Comment 21

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1160361 - Move commands-index to the server. r=bgrins

Comment 22

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1160361 - Catch GCLI's load of definitions for non-Firefox. r=bgrins

Comment 23

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1160361 - Move gclicommands.properties to toolkit. r=bgrins

Comment 24

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1160361 - Move gcli.properties to toolkit. r=bgrins

Comment 25

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1160361 - Move GCLI prefs to all.js. r=bgrins

Comment 26

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1160361 - Load tilt commands from ToolboxButtons to avoid in non-Firefox. r=bgrins

Comment 27

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1160361 - Skip Telemetry calls for non-Firefox. r=bgrins

Comment 28

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1160361 - Abort tilt commands when remote. r=bgrins