1252980 - Add servo linkage glue into mozilla-central

Status: RESOLVED FIXED

Description

[Bobby Holley (:bholley)]

This will remove some of the current hackiness in our branch.

Comment 1

[Bobby Holley (:bholley)]

Created attachment 8725824 [details] [diff] [review]
Link the geckolib into libxul and define MOZ_STYLO if --with-servo=PATH is passed. v1

Comment 2

[Bobby Holley (:bholley)]

Looks green.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=880f1e7f2a96

Comment 3

[Gregory Szorc [:gps]]

Comment on attachment 8725824 [details] [diff] [review]
Link the geckolib into libxul and define MOZ_STYLO if --with-servo=PATH is passed. v1

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

::: old-configure.in
@@ +7601,5 @@
>  dnl ========================================================
> +dnl = Use the Servo Style System for Gecko.
> +dnl ========================================================
> +MOZ_ARG_WITH_STRING(servo,
> +[  --with-servo=SERVO_TARGET_DIR

--with-servo feels ambiguous. Let's change this to what it is: the path to Servo's libraries. 

--with-servo-libs-dir=path

(bikeshedding welcome - there is no shortage of naming conventions for "dirs" "libs" and "path" in configure :/ )

@@ +7607,5 @@
> +                be found. This is generally servo_src_dir/target/release],
> +  SERVO_TARGET_DIR=$withval,
> +  SERVO_TARGET_DIR=)
> +if test -n "$SERVO_TARGET_DIR"; then
> +   AC_DEFINE(MOZ_STYLO)

You may not be able to answer this today, but should --with-servo-libs-dir imply enabling *all* the Servo-based features? Or should we be creating --enable-<feature> flags for every Servo based component so developers can cherry pick?

::: toolkit/library/moz.build
@@ +27,5 @@
>          'winspool.drv'
>      ]
>  
> +    if CONFIG['SERVO_TARGET_DIR']:
> +        OS_LIBS += ['-L' + CONFIG['SERVO_TARGET_DIR'], '-lgeckoservo']

The preferred pattern is to AC_SUBST a variable containing "-L{path}" and to do "OS_LIBS += [CONFIG['FOO_LIBS']]" Please do that instead.

Comment 4

[Bobby Holley (:bholley)]

Created attachment 8725972 [details] [diff] [review]
Link the geckolib into libxul and define MOZ_STYLO if --with-servo=PATH is passed. v2

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8725972 [details] [diff] [review]
Link the geckolib into libxul and define MOZ_STYLO if --with-servo=PATH is passed. v2

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

LGTM.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4f60d9123df

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b4f60d9123df