Skip to content

Update to use latest xlink repo and added Clang support#1094

Open
onthegrid007 wants to merge 7 commits into
luxonis:developfrom
onthegrid007:develop
Open

Update to use latest xlink repo and added Clang support#1094
onthegrid007 wants to merge 7 commits into
luxonis:developfrom
onthegrid007:develop

Conversation

@onthegrid007
Copy link
Copy Markdown

Update to use latest xlink repo and added Clang support

@moratom
Copy link
Copy Markdown
Collaborator

moratom commented Aug 12, 2024

Thanks @onthegrid007 - any particular reason for the switch to ninja?

@moratom
Copy link
Copy Markdown
Collaborator

moratom commented Aug 12, 2024

CI fails on MacOS: https://github.com/luxonis/depthai-core/actions/runs/10337442673/job/28646392973

Let's either leave the CI builders as is or extend with a "matrix" to build with both GCC and Clang.

@onthegrid007
Copy link
Copy Markdown
Author

Thanks @onthegrid007 - any particular reason for the switch to ninja?

Ninja is required for Clang on windows to overwrite the use of MSVS compiler in Cmake...

Thought it would be easier overall if everything just switched over at once.

@onthegrid007
Copy link
Copy Markdown
Author

Unfortunately unless I rebuild opencv in the ci I have to use clang with a target for mingw and mingw be the library target on windows, Ninja is still required for it not to default to MSVS, and since it's only a single binary for installation I'm not too worried...

Question, @moratom should I rebuild opencv cleanly in the ci so clang can work without mingw (see windows latest build for the error I'm mentioning) or is there a way like it kind of mentions to just specify opencv_dir and hope there's no binary discrepancies between the mingw prebuilt opencv binaries and clang compiling with msvs as the target libraries?

@moratom
Copy link
Copy Markdown
Collaborator

moratom commented Aug 18, 2024

@onthegrid007 we don't necessarily want to switch the CI to clang, rather we should extend it, so we run both with GCC and clang in the CI.

You can use the "matrix" for that in github actions.

Note that we'll be mainlining the updated XLink in this PR #1098 and keep the this one to expend the CI to clang.

@themarpe themarpe requested a review from moratom September 3, 2024 14:24
@onthegrid007
Copy link
Copy Markdown
Author

Is this good enough?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants