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

Windows: Add missing dependencies #21968

Merged
merged 5 commits into from Feb 2, 2019
Merged

Windows: Add missing dependencies #21968

merged 5 commits into from Feb 2, 2019

Conversation

@UK992
Copy link
Contributor

UK992 commented Oct 16, 2018

Rebased #16445 and updated with Gstreamer DLLs.

About msi installer, there is also included gstreamer installer, should be removed and replaced by needed gstreamer DLLs or keep it at is it?

Fixes #16422.


This change is Reviewable

@highfive
Copy link

highfive commented Oct 16, 2018

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/servo/command_base.py, python/servo/build_commands.py, python/servo/package_commands.py
@jdm
Copy link
Member

jdm commented Nov 27, 2018

@bors-servo r+
Looks good! Sorry for the delay in reviewing these changes.

@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2018

📌 Commit a31c8d7 has been approved by jdm

@highfive highfive assigned jdm and unassigned ferjm Nov 27, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2018

Testing commit a31c8d7 with merge 02d30fa...

bors-servo added a commit that referenced this pull request Nov 27, 2018
Windows: Add missing dependencies

Rebased #16445 and updated with Gstreamer DLLs.

About msi installer, there is also included gstreamer installer, should be removed and replaced by needed gstreamer DLLs or keep it at is it?

<!-- 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/21968)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2018

💔 Test failed - windows-msvc-dev

@jdm
Copy link
Member

jdm commented Nov 27, 2018

Error running mach:

    ['build', '--dev']

The error occurred in the implementation of the invoked mach command.

This should never occur and is likely a bug in the implementation of that
command. Consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

KeyError: u'GSTREAMER_1_0_ROOT_X86_64'

  File "c:\buildbot\windows-msvc-dev\build\python\servo\build_commands.py", line 591, in build
    gst_root = env["GSTREAMER_1_0_ROOT_" + gst_x64]
program finished with exit code 1

Where is that environment variable supposed to come from?

@UK992
Copy link
Contributor Author

UK992 commented Dec 1, 2018

@jdm Gstreamer installer set this variable.

@UK992 UK992 force-pushed the UK992:mach-package branch from a31c8d7 to 75aa241 Dec 1, 2018
@atouchet
Copy link
Contributor

atouchet commented Dec 1, 2018

Would this help with #22315?

@UK992
Copy link
Contributor Author

UK992 commented Dec 1, 2018

@atouchet it should solve this issue.

Also I removed GStreamer installer from package, because it is no longer needed.

@UK992 UK992 force-pushed the UK992:mach-package branch from 2ee2223 to d929428 Dec 1, 2018
@atouchet
Copy link
Contributor

atouchet commented Dec 8, 2018

@jdm r?

@jdm
Copy link
Member

jdm commented Dec 13, 2018

Ok, so let's be clear - the effect of this change is that:

  • we generate an EXE instead of an MSI
  • we no longer run the official GStreamer installer during the installation
  • the installer contains all of the known DLL dependencies of Servo, including the GStreamer ones

Is that correct?

bors-servo added a commit that referenced this pull request Feb 1, 2019
Windows: Add missing dependencies

Rebased #16445 and updated with Gstreamer DLLs.

About msi installer, there is also included gstreamer installer, should be removed and replaced by needed gstreamer DLLs or keep it at is it?

Fixes #16422.

<!-- 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/21968)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 1, 2019

💔 Test failed - status-taskcluster

@UK992
Copy link
Contributor Author

UK992 commented Feb 1, 2019

@jdm try again please

Added missing gstreamer-1.0-x86_64-1.14.3.msi which contain needed dlls.

@jdm
Copy link
Member

jdm commented Feb 1, 2019

@bors-servo try=windows

@bors-servo
Copy link
Contributor

bors-servo commented Feb 1, 2019

Trying commit dc2cc29 with merge e21757f...

bors-servo added a commit that referenced this pull request Feb 1, 2019
Windows: Add missing dependencies

Rebased #16445 and updated with Gstreamer DLLs.

About msi installer, there is also included gstreamer installer, should be removed and replaced by needed gstreamer DLLs or keep it at is it?

Fixes #16422.

<!-- 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/21968)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 1, 2019

💔 Test failed - status-taskcluster

@UK992 UK992 force-pushed the UK992:mach-package branch from dc2cc29 to f374dc3 Feb 1, 2019
@highfive highfive removed the S-tests-failed label Feb 1, 2019
@UK992
Copy link
Contributor Author

UK992 commented Feb 1, 2019

Installing gstreamer-1.0-x86_64-1.14.3.msi and setting GSTREAMER_1_0_ROOT_X86_64 environment variable seems to solve the problem with Gstreamer.

But adding another .with_repacked_msi() in etc/taskcluster/decision_task.py creates new task in TC.

@jdm
Copy link
Member

jdm commented Feb 1, 2019

@bors-servo r+
LGTM! Thanks for fixing that!

@SimonSapin
Copy link
Member

SimonSapin commented Feb 1, 2019

Creating new tasks is ok, and output of the MSI ones is cached so they’ll only run again when caches expire.

@atouchet
Copy link
Contributor

atouchet commented Feb 1, 2019

@jdm I don't think that worked. Retry?

@bors-servo
Copy link
Contributor

bors-servo commented Feb 1, 2019

📌 Commit f374dc3 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Feb 1, 2019

Testing commit f374dc3 with merge 3630735...

bors-servo added a commit that referenced this pull request Feb 1, 2019
Windows: Add missing dependencies

Rebased #16445 and updated with Gstreamer DLLs.

About msi installer, there is also included gstreamer installer, should be removed and replaced by needed gstreamer DLLs or keep it at is it?

Fixes #16422.

<!-- 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/21968)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 2, 2019

@bors-servo bors-servo merged commit f374dc3 into servo:master Feb 2, 2019
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
@atouchet
Copy link
Contributor

atouchet commented Feb 2, 2019

I tried running the latest nightly without having GStreamer installed and got some errors about missing DLLs.

Installer

@jdm
Copy link
Member

jdm commented Feb 2, 2019

Those will need to be added to the list of included DLLs.

bors-servo added a commit that referenced this pull request Feb 2, 2019
Windows: Add more missing dependencies

From #21968 (comment)
r? @jdm

cc @atouchet

<!-- 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/22811)
<!-- Reviewable:end -->
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.

None yet

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