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

Fix various Windows dependency issues #23126

Merged
merged 3 commits into from Apr 1, 2019

Conversation

Projects
None yet
7 participants
@jdm
Copy link
Member

commented Mar 27, 2019

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #23125 and fix #23104
  • There are tests for these changes

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

commented Mar 27, 2019

Heads up! This PR modifies the following files:

@jdm

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

I'm going to see what appveyor thinks of these changes.

@CYBAI

This comment has been minimized.

Copy link
Collaborator

commented Mar 28, 2019

Not sure if we need to change this line as well?

if os.path.exists(path.join(gst_default_path, "bin", "libz.dll")):

Btw, not sure if it's better to detect gstreamer version and trim the lib prefix only for version higher than 1.15.2 ? 🤔

@jdm

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

Good catch.

@jdm

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

r? @UK992

@jdm

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

@highfive highfive assigned Manishearth and unassigned SimonSapin Mar 29, 2019

@UK992

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2019

I have tested with both 1.15.1 and 1.15.2, and it works as expected.

@Manishearth
Copy link
Member

left a comment

r=me

]
if gst_root:
for gst_lib in gst_dlls:
shutil.copy(path.join(gst_root, "bin", gst_lib),
servo_exe_dir)
if isinstance(gst_lib, str):

This comment has been minimized.

Copy link
@Manishearth

Manishearth Apr 1, 2019

Member

this pair of lines could be removed, but it's probably fine to keep them around

@jdm

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

@bors-servo r=Manishearth

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

📌 Commit 73aeee1 has been approved by Manishearth

bors-servo added a commit that referenced this pull request Apr 1, 2019

Auto merge of #23126 - servo:jdm-patch-35, r=Manishearth
Fix various Windows dependency issues

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #23125 and fix #23104
- [x] There are tests for these changes

<!-- 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/23126)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

⌛️ Testing commit 73aeee1 with merge e27653c...

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

💔 Test failed - mac-rel-css2

@jdm

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

@bors-servo bors-servo merged commit 73aeee1 into master Apr 1, 2019

4 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.