1208112 - Package DevTools client in SeaMonkey now that it's been moved out of /browser/

Status: RESOLVED FIXED

(SeaMonkey :: Installer, defect)

Description

[Philip Chee]

q.v. Bug 912121 - Move DevTools code to /devtools top-level directory

Comment 1

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

It may be a little early for this step still?  We still make use of some files from the /browser directory today inside /devtools/client.

Comment 2

[Philip Chee]

Attachment: Patch v1.0 Package fix.

Comment 3

[Philip Chee]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #1)
> It may be a little early for this step still?  We still make use of some
> files from the /browser directory today inside /devtools/client.
I'm just packaging it at the moment. I'm not going to hook it up to the UI yet.

> +++ b/suite/moz.build
....
> +    '/mozilla/devtools/client',
When I do this the client part goes into omni.ja!/modules/mozilla/devtools/client/ when it should go into /modules/devtools/client/
Any advice?

Comment 4

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

(In reply to Philip Chee from comment #3)
> > +++ b/suite/moz.build
> ....
> > +    '/mozilla/devtools/client',
> When I do this the client part goes into
> omni.ja!/modules/mozilla/devtools/client/ when it should go into
> /modules/devtools/client/
> Any advice?

Hmm.  With Firefox, the files are installed to <obj>/dist/bin/browser/modules/devtools/client.

They then become part of the browser/omni.ja!modules/devtools/client.

AIUI, this works because browser/moz.build includes:

  DIST_SUBDIR = 'browser'
  export('DIST_SUBDIR')

Do you have a similar thing?

Comment 5

[Philip Chee]

Attachment: moz.build Patch for mozilla-central

> AIUI, this works because browser/moz.build includes:

>   DIST_SUBDIR = 'browser'
>   export('DIST_SUBDIR')

> Do you have a similar thing?
No we don't and that's the problem. Firefox has a mezzanine /browser/ level and the Firefox devtools code is spread over two omni.ja's. SeaMonkey has one unified omni.ja without the intermediate level[1].

[1] This was created so that Desktop Firefox and Metrofox could be shipped in the same package. Now that the Metro code has been removed the /browser/ is redundant and the two omni.ja's can be re-united. But that's grist for another bug really.

CC a build expert for some advice.

Comment 6

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

Comment on attachment 8666375 [details] [diff] [review]
moz.build Patch for mozilla-central

Review of attachment 8666375 [details] [diff] [review]:
-----------------------------------------------------------------

I don't have a strong opinion here since it's guarded by MOZ_SUITE.

I'll leave it up to the build peers.

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8666375 [details] [diff] [review]
moz.build Patch for mozilla-central

Review of attachment 8666375 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/toolkit.mozbuild
@@ +151,5 @@
>      '/other-licenses/snappy',
>  ]
>  
> +if CONFIG['MOZ_SUITE']:
> +    DIRS += ['/devtools/client']

You should just add this to some suite/ moz.build.

Comment 8

[Philip Chee]

>> +if CONFIG['MOZ_SUITE']:
>> +    DIRS += ['/devtools/client']
> 
> You should just add this to some suite/ moz.build.
How do I do this without running into the problem in Comment 3 ?

Comment 9

[Mike Hommey [:glandium]]

DIRS += ['/devtools/client'] somewhere in a suite/ moz.build

Comment 10

[Philip Chee]

(In reply to Mike Hommey [:glandium] from comment #9)
> DIRS += ['/devtools/client'] somewhere in a suite/ moz.build

The error occurred while processing the following file:
    c:/t1/hg/comm-central/suite/moz.build
The underlying problem is we referenced a path that does not exist. That path is:
    c:/t1/hg/comm-central/devtools/client/moz.build
Either create the file if it needs to exist or do not reference it.

Got a better suggestion?

Comment 11

[Philip Chee]

(In reply to Mike Hommey [:glandium] from comment #9)
> DIRS += ['/devtools/client'] somewhere in a suite/ moz.build

The error occurred while processing the following file:
    c:/t1/hg/comm-central/suite/moz.build
The underlying problem is we referenced a path that does not exist. That path is:
    c:/t1/hg/comm-central/devtools/client/moz.build
Either create the file if it needs to exist or do not reference it.

Got a better suggestion?

Comment 12

[Philip Chee]

Putting it in ** app.mozbuild ** seems to work

Comment 13

[Philip Chee]

Attachment: Patch v2.0 use app.mozbuild instead

This packages the Devtools Client which has been moved out of /browser/ into a new top level directory. It isn't usable at the moment since the L10n files are still in /browser/

Comment 14

[Philip Chee]

Attachment: Patch v2.1 Include the devtools preferences file

Comment 15

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

Bug 1203159 has changed this up a bit.

You would now do something like:

app.mozbuild:

DIRS += ['/devtools']

confvars.sh:

MOZ_DEVTOOLS=all

Comment 16

[Philip Chee]

Comment on attachment 8675296 [details] [diff] [review]
Patch v2.1 Include the devtools preferences file

Need to fix bitrot

Comment 17

[Philip Chee]

Attachment: Patch v3 updated to take into account Bug 1203159

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #15)
> Bug 1203159 has changed this up a bit.
> 
> You would now do something like:
> 
> app.mozbuild:
> 
> DIRS += ['/devtools']
> 
> confvars.sh:
> 
> MOZ_DEVTOOLS=all

Thanks!

Comment 18

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

Unfortunately things changed slightly again in bug 1217687, where we tried to simplify this a bit for all apps.  This has only landed to fx-team so far, should merge to m-c soon.

/toolkit now does the DIRS += ['/devtools'] step, so you no longer want this in your app.mozbuild (assuming in includes toolkit/toolkit.mozbuild already).

So, I would guess remove that part, but otherwise it should be good.

Comment 19

[Philip Chee]

Attachment: Patch v4

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #18)
> Unfortunately things changed slightly again in bug 1217687, where we tried
> to simplify this a bit for all apps.  This has only landed to fx-team so
> far, should merge to m-c soon.
> 
> /toolkit now does the DIRS += ['/devtools'] step, so you no longer want this
> in your app.mozbuild (assuming in includes toolkit/toolkit.mozbuild already).
> 
> So, I would guess remove that part, but otherwise it should be good.
Done.

Comment 20

[Philip Chee]

http://hg.mozilla.org/comm-central/rev/ba4137b57c77

Comment 21

[Philip Chee]

Comment on attachment 8678568 [details] [diff] [review]
Patch v4

A followup patch is needed. Package the devtools.js preference file only if the Devtools client is built and packaged. The devtools manifest should be packaged for all values of MOZ_DEVTOOLS


 ; DevTools
 @RESPATH@/chrome/devtools@JAREXT@
 @RESPATH@/chrome/devtools.manifest
+#if MOZ_DEVTOOLS == all
 @RESPATH@/@PREF_DIR@/devtools.js
+#endif