Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Isolate UWP builds from external gstreamers #24343

Merged
merged 2 commits into from Oct 2, 2019

Conversation

@Manishearth
Copy link
Member

Manishearth commented Oct 2, 2019

Fixes #24327

This further seals the UWP build from the non-cross regular environment and makes pkg-config look at the UWP package.

This makes sure external gstreamer stuff doesn't sneak in. We already kinda do this by setting the LIB environment variable, but PKG_CONFIG_PATH also sneaks in and causes problems. Thing is, it turns out that the pkgconfig in gstreamer-uwp isn't enough for a full servo build, since we don't package gstreamer-webrtc, and the gstreamer-webrtc crate requires it to be around, even if we won't end up loading the library at runtime. Stuff has succeeded so far because people have gstreamer installations whose PKG_CONFIG_PATH is pulled in, despite us using a different set of DLLs, which somehow works but sometimes doesn't (I still don't know why).

I've added a fake gstreamer-webrtc-1.0.pc file to both targets in the gstreamer-uwp package with the following contents. It doesn't do anything the other pc files don't, so it doesn't end up pulling in additional libraries, it just exists to convince pkgconfig that we have this library (even though we don't), so that the build may succeed (we'll fail at runtime when we try to open WebRTC connections, but those are disabled anyway).


prefix=c:/gstreamer/1.0/arm64.uwp-release
exec_prefix=${prefix}
libdir=${prefix}/lib
includedir=${prefix}/include/gstreamer-1.0
datarootdir=${prefix}/share
datadir=${datarootdir}
girdir=${datadir}/gir-1.0
typelibdir=${libdir}/girepository-1.0

Name: Fake GStreamer WebRTC library
Description: Hacky stand-in for gstreamer-webrtc, does nothing but stops the Rust gstreamer-webrtc crate from failing during build
Requires: gstreamer-1.0 gstreamer-base-1.0
Version: 1.16.0
Libs: -L${libdir}
Cflags: -I${includedir}

r? @jdm

Perhaps don't merge yet, I can't test this until tomorrow (and ideally would like to test this from scratch tomorrow evening)


This change is Reviewable

This new package contains a hacky gstreamer-webrtc-1.0.pc file so that we can still do self-contained builds without relying on external PKG_CONFIG_PATH.
@highfive
Copy link

highfive commented Oct 2, 2019

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/servo/build_commands.py, python/servo/packages.py
@jdm
jdm approved these changes Oct 2, 2019
@jdm jdm added S-fails-tidy and removed S-awaiting-review labels Oct 2, 2019
@Manishearth
Copy link
Member Author

Manishearth commented Oct 2, 2019

python/servo/build_commands.py Outdated Show resolved Hide resolved
@Manishearth Manishearth force-pushed the Manishearth:pkg-config-gst-uwp branch from fae4c94 to c716d46 Oct 2, 2019
@ferjm
Copy link
Member

ferjm commented Oct 2, 2019

Could we generate the fake gstreamer-webrtc-1.0.pc from Servo so we don't need to remember to add it when generating new binaries?

@Manishearth
Copy link
Member Author

Manishearth commented Oct 2, 2019

I'd rather document this since this is temporary: is there a place where we document how this package is generated?

@ferjm
Copy link
Member

ferjm commented Oct 2, 2019

Ok.
There is no documentation yet. We have been using https://nirbheek.in/files/mozilla/uwp/gstreamer/0.4/ binaries so far. But these are missing the EGL related symbols that we need, so I am preparing a new version of the binaries (I got the x64 version so far, trying to figure out the cross compilation part). I will document the process when done and I'll add the fake webrtc bits there.

@paulrouget
Copy link
Contributor

paulrouget commented Oct 2, 2019

Builds and runs well here.

@jdm
Copy link
Member

jdm commented Oct 2, 2019

This can merge when you're happy with it.

@Manishearth
Copy link
Member Author

Manishearth commented Oct 2, 2019

@bors r=jdm

@Manishearth
Copy link
Member Author

Manishearth commented Oct 2, 2019

@bors-servo r=jdm

@bors-servo
Copy link
Contributor

bors-servo commented Oct 2, 2019

📌 Commit c716d46 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Oct 2, 2019

Testing commit c716d46 with merge 22e3797...

bors-servo added a commit that referenced this pull request Oct 2, 2019
Isolate UWP builds from external gstreamers

Fixes #24327

This further seals the UWP build from the non-cross regular environment and makes pkg-config look at the UWP package.

This makes sure external gstreamer stuff doesn't sneak in. We already kinda do this by setting the LIB environment variable, but PKG_CONFIG_PATH also sneaks in and causes problems. Thing is, it turns out that the pkgconfig in gstreamer-uwp isn't enough for a full servo build, since we don't package gstreamer-webrtc, and the gstreamer-webrtc crate requires it to be around, even if we won't end up loading the library at runtime. Stuff has succeeded so far because people have gstreamer installations whose PKG_CONFIG_PATH is pulled in, despite us using a different set of DLLs, which somehow works but sometimes doesn't (I still don't know why).

I've added a fake gstreamer-webrtc-1.0.pc file to both targets in the gstreamer-uwp package with the following contents. It doesn't do anything the other pc files don't, so it doesn't end up pulling in additional libraries, it just exists to convince pkgconfig that we *have* this library (even though we don't), so that the build may succeed (we'll fail at runtime when we try to open WebRTC connections, but those are disabled anyway).

<details>

```pkgconfig

prefix=c:/gstreamer/1.0/arm64.uwp-release
exec_prefix=${prefix}
libdir=${prefix}/lib
includedir=${prefix}/include/gstreamer-1.0
datarootdir=${prefix}/share
datadir=${datarootdir}
girdir=${datadir}/gir-1.0
typelibdir=${libdir}/girepository-1.0

Name: Fake GStreamer WebRTC library
Description: Hacky stand-in for gstreamer-webrtc, does nothing but stops the Rust gstreamer-webrtc crate from failing during build
Requires: gstreamer-1.0 gstreamer-base-1.0
Version: 1.16.0
Libs: -L${libdir}
Cflags: -I${includedir}

```
</details>

r? @jdm

Perhaps don't merge yet, I can't test this until tomorrow (and ideally would like to test this from scratch tomorrow evening)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24343)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 2, 2019

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: jdm
Pushing 22e3797 to master...

@bors-servo bors-servo merged commit c716d46 into servo:master Oct 2, 2019
3 checks passed
3 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Oct 2, 2019
4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants
You can’t perform that action at this time.