WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
150849
Teach Makefile to build LayoutTestRelay when building for iOS Simulator
https://bugs.webkit.org/show_bug.cgi?id=150849
Summary
Teach Makefile to build LayoutTestRelay when building for iOS Simulator
Daniel Bates
Reported
2015-11-03 12:43:24 PST
For convenience, we should support building LayoutTestRelay by default when building WebKit using `make SDKROOT=iphonesimulator`.
Attachments
Patch
(1.88 KB, patch)
2015-11-03 12:47 PST
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Patch
(3.07 KB, patch)
2015-11-05 17:38 PST
,
Daniel Bates
ap
: review+
Details
Formatted Diff
Diff
[Patch] Addendum - Build LayoutTestRelay when make invoked from Tools or top-level checkout
(1.59 KB, patch)
2015-11-10 20:55 PST
,
Daniel Bates
ap
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Daniel Bates
Comment 1
2015-11-03 12:47:08 PST
Created
attachment 264714
[details]
Patch
Alexey Proskuryakov
Comment 2
2015-11-03 13:32:55 PST
Comment on
attachment 264714
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=264714&action=review
> Tools/LayoutTestRelay/Makefile:14 > +ifneq (,$(SAVED_SDK_ROOT)) > + override SDKROOT = $(SAVED_SDK_ROOT) > +endif
Should we also unset SAVED_SDK_ROOT here?
Daniel Bates
Comment 3
2015-11-05 13:57:38 PST
(In reply to
comment #2
)
> Comment on
attachment 264714
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=264714&action=review
> > > Tools/LayoutTestRelay/Makefile:14 > > +ifneq (,$(SAVED_SDK_ROOT)) > > + override SDKROOT = $(SAVED_SDK_ROOT) > > +endif > > Should we also unset SAVED_SDK_ROOT here?
Yes, will modify patch to undefine SAVED_SDK_ROOT inside the if block above before landing.
Daniel Bates
Comment 4
2015-11-05 14:01:11 PST
Committed
r192073
: <
http://trac.webkit.org/changeset/192073
>
WebKit Commit Bot
Comment 5
2015-11-05 15:14:47 PST
Re-opened since this is blocked by
bug 150962
Daniel Bates
Comment 6
2015-11-05 17:38:43 PST
Created
attachment 264909
[details]
Patch
Daniel Bates
Comment 7
2015-11-06 08:59:27 PST
Committed
r192106
: <
http://trac.webkit.org/changeset/192106
>
Jason Marcell
Comment 8
2015-11-06 14:43:45 PST
Comment on
attachment 264909
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=264909&action=review
> Tools/ChangeLog:11 > + * LayoutTestRelay/Makefile: Define OVERWRITE_SDKROOT and OVERWRITE_ARCHS.
Nit: OVERWRITE_{ARCHS, SDKROOT} -> OVERRIDE_{ARCHS, SDKROOT}
> Makefile.shared:7 > + ifneq (,$(OVERRIDE_SDKROOT))
Why is this nested inside the first if block? Don't you want to use the OVERRIDE_SDKROOT regardless of whether SDKROOT was set?
> Makefile.shared:9 > + XCODE_OPTIONS := $(XCODE_OPTIONS) SDKROOT=$(OVERRIDE_SDKROOT)
Why did you use this syntax instead of the += syntax that was previously used?
Jason Marcell
Comment 9
2015-11-06 14:43:54 PST
Comment on
attachment 264909
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=264909&action=review
> Tools/ChangeLog:11 > + * LayoutTestRelay/Makefile: Define OVERWRITE_SDKROOT and OVERWRITE_ARCHS.
Nit: OVERWRITE_{ARCHS, SDKROOT} -> OVERRIDE_{ARCHS, SDKROOT}
> Makefile.shared:7 > + ifneq (,$(OVERRIDE_SDKROOT))
Why is this nested inside the first if block? Don't you want to use the OVERRIDE_SDKROOT regardless of whether SDKROOT was set?
> Makefile.shared:9 > + XCODE_OPTIONS := $(XCODE_OPTIONS) SDKROOT=$(OVERRIDE_SDKROOT)
Why did you use this syntax instead of the += syntax that was previously used?
Daniel Bates
Comment 10
2015-11-10 20:40:45 PST
(In reply to
comment #8
)
> Comment on
attachment 264909
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=264909&action=review
> > > Tools/ChangeLog:11 > > + * LayoutTestRelay/Makefile: Define OVERWRITE_SDKROOT and OVERWRITE_ARCHS. > > Nit: OVERWRITE_{ARCHS, SDKROOT} -> OVERRIDE_{ARCHS, SDKROOT} >
Fixed in <
http://trac.webkit.org/changeset/192289
>.
> > Makefile.shared:7 > > + ifneq (,$(OVERRIDE_SDKROOT)) > > Why is this nested inside the first if block? Don't you want to use the > OVERRIDE_SDKROOT regardless of whether SDKROOT was set? >
Yes, we should respect OVERRIDE_SDKROOT regardless of whether SDKROOT is set. Filed
bug #151127
to address this issue.
> > Makefile.shared:9 > > + XCODE_OPTIONS := $(XCODE_OPTIONS) SDKROOT=$(OVERRIDE_SDKROOT) > > Why did you use this syntax instead of the += syntax that was previously > used?
Notice that XCODE_OPTIONS is a recursively expanded variable by <
https://ftp.gnu.org/old-gnu/Manuals/make-3.79.1/html_chapter/make_6.html#SEC59
>. The += operator preserves the flavor of the variable by <
https://ftp.gnu.org/old-gnu/Manuals/make-3.79.1/html_chapter/make_6.html#SEC65
>. Due to the way Makefiles are executed, we want to expand $OVERRIDE_SDKROOT in the definition of XCODE_OPTIONS immediately (i.e. make XCODE_OPTIONS a simply expanded variable) so that we can clear it and allow subsequent Makefiles to define it again. Therefore, I used the := operator to define XCODE_OPTIONS as a simply-expanded variable. For some reason, I was concerned about unconditionally defining XCODE_OPTIONS as a simply expanded variable and chose to only convert XCODE_OPTIONS to a simply-expanded variable when $(OVERRIDE_SDKROOT) is defined. We should look to unconditionally define XCODE_OPTIONS as a simply expanded variable and then we can use the += operator when appending the expression "SDKROOT=$(OVERRIDE_SDKROOT)".
Daniel Bates
Comment 11
2015-11-10 20:45:01 PST
Re-opening this bug since I inadvertently did not append LayoutTestRelay to the end of the list of modules to build in file Tools/Makefile. So, `make SDKROOT=iphonesimulator` only works when executed inside directory Tools/LayoutTestRelay. We should make `make SDKROOT=iphonesimulator` work when executed from directory Tools and the top-level WebKit checkout.
Daniel Bates
Comment 12
2015-11-10 20:55:24 PST
Created
attachment 265266
[details]
[Patch] Addendum - Build LayoutTestRelay when make invoked from Tools or top-level checkout
Daniel Bates
Comment 13
2015-11-10 21:19:58 PST
Committed
r192293
: <
http://trac.webkit.org/changeset/192293
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug