Skip to content
This repository has been archived by the owner on Jan 18, 2022. It is now read-only.

ci: pass tests on Windows #49

Merged
merged 10 commits into from
Oct 11, 2021
Merged

ci: pass tests on Windows #49

merged 10 commits into from
Oct 11, 2021

Conversation

homuler
Copy link
Contributor

@homuler homuler commented Oct 11, 2021

closes #23

This PR enables us to build libmediapipe_c.so with --opencv cmake option on Windows.
Disabled tests on Windows CI are restored, too.

In order to pass all tests, I also fixed:

  • DllNotFoundException that occurs when any native APIs are called.
    • With [DllImport('mediapipe_c')], it seems that Windows loader cannot find libmediapipe_c.dll.
  • Packet#DebugTypeName returns different results than when run on Linux.

By the way, I heard that EmguCV (OpenCV 4.x) is used in this project.
If so, I think it's not ideal to link OpenCV (3.4.10) statically, and libmediapipe_c.dll should link to the OpenCV library that EmguCV uses.

Major Changes

  • Modify end_of_line in .editorconfig to match the current newline of most files.
    • But ci.yml uses CR+LF, and diffs got larger.
  • Run tests on Windows
  • Enable to build OpenCV and link it statically to libmediapipe_c.so on Windows (i.e. --opencv cmake).
  • Fix tests that fail on Windows
  • Fix DllNotFoundException that occurs when it's run on Windows

Notes

  • On Windows, bazel's output folder (i.e. C:\_bzl) isn't still cached.
  • WITH_LAPACK option is disabled, because it can take +20 minutes more than without it on CI.

@sr229 sr229 added the enhancement New feature or request label Oct 11, 2021
@sr229 sr229 linked an issue Oct 11, 2021 that may be closed by this pull request
@homuler homuler marked this pull request as ready for review October 11, 2021 14:34
@sr229 sr229 added the hacktoberfest Get ready for Hacktoberfest 2021! Issues labeled this is a Hacktoberfest PR/Issue label Oct 11, 2021
Copy link
Member

@sr229 sr229 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, oh this is a Hacktoberfest issue, so if you ever feel like getting a T-shirt, this counts!

@sr229 sr229 merged commit c0fbc4a into vignetteapp:master Oct 11, 2021
Speykious pushed a commit that referenced this pull request Oct 11, 2021
* build: copy opencv.BUILD from MediaPipeUnityPlugin

* chore: current end_of_line is LF

* build: remove opencv_deps option

* ci: enable Windows

* build: pass environment variables to build OpenCV

* ci: disable LAPACK

* ci: ignore SignalAbortTest on Windows

* test: fix tests on Windows

* doc: remove an incorrect line

* Do not upload contents of build/

We already have Bazel Caching so I doubt we even needed this.

Co-authored-by: Capuccino <ayane@vignetteapp.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request hacktoberfest Get ready for Hacktoberfest 2021! Issues labeled this is a Hacktoberfest PR/Issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Windows CI Accomodate multi-platform building on CI
2 participants