Add servo linkage glue into mozilla-central

RESOLVED FIXED in Firefox 47

Status

Firefox Build System
General
RESOLVED FIXED
2 years ago
5 months ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

(Blocks: 1 bug)

unspecified
mozilla47

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
This will remove some of the current hackiness in our branch.
(Assignee)

Comment 1

2 years ago
Created attachment 8725824 [details] [diff] [review]
Link the geckolib into libxul and define MOZ_STYLO if --with-servo=PATH is passed. v1
Attachment #8725824 - Flags: review?(gps)

Comment 3

2 years ago
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)
(Assignee)

Comment 4

2 years ago
Created attachment 8725972 [details] [diff] [review]
Link the geckolib into libxul and define MOZ_STYLO if --with-servo=PATH is passed. v2
Attachment #8725972 - Flags: review?(gps)
(Assignee)

Updated

2 years ago
Attachment #8725824 - Attachment is obsolete: true

Comment 5

2 years ago
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+

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b4f60d9123df
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47

Updated

5 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.