Closed Bug 1252980 Opened 4 years ago Closed 4 years ago

Add servo linkage glue into mozilla-central

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox47 fixed)

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

Details

Attachments

(1 file, 1 obsolete file)

This will remove some of the current hackiness in our branch.
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.
Attachment #8725824 - Flags: review?(gps)
Attachment #8725824 - Attachment is obsolete: true
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.
Attachment #8725972 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/b4f60d9123df
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.