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

Clang Support #84

Merged
merged 10 commits into from
Jul 31, 2024
Merged

Clang Support #84

merged 10 commits into from
Jul 31, 2024

Conversation

onthegrid007
Copy link
Contributor

Adds Clang support
Fixes issues:
#80
luxonis/depthai-core#1044

CMakeLists.txt Outdated Show resolved Hide resolved
@onthegrid007
Copy link
Contributor Author

Could this get pushed downstream so depthai-core can use it properly?

CMakeLists.txt Outdated Show resolved Hide resolved
@onthegrid007
Copy link
Contributor Author

Done.

Copy link
Collaborator

@themarpe themarpe left a comment

Choose a reason for hiding this comment

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

@onthegrid007 sorry, one additional thing - do you mind adding it to CI as well?

@themarpe
Copy link
Collaborator

(otherwise LGTM)

@onthegrid007
Copy link
Contributor Author

I can if you could tell me what the CI is...?

@themarpe
Copy link
Collaborator

@onthegrid007 the Continious Integration - there is a .yml workflow file that runs "tests/builds" of the codebase using various platforms/operating systems. windows-latest currently only does MSVC, so we should add also Clang there

@onthegrid007
Copy link
Contributor Author

Yes I can try. Give ma little bit

@onthegrid007
Copy link
Contributor Author

Done.

@themarpe
Copy link
Collaborator

@onthegrid007
Quickly checking the job: https://github.com/luxonis/XLink/actions/runs/10069049885/job/27853635084?pr=84 it doesn't seem like clang is picked up. Try either changing the generator or use a toolchain file to set compiler

@onthegrid007
Copy link
Contributor Author

It succeeded...

@themarpe
Copy link
Collaborator

I mean that clang wasn't used to compile, but it was still MSVC:

-- The CXX compiler identification is MSVC 19.40.33812.0

Under the windows, clang build

@onthegrid007
Copy link
Contributor Author

I'll make a change and see if it fixes it

@onthegrid007
Copy link
Contributor Author

Ninja or some other makefile system is required for cmake to execute using clang properly, that has been added and should allow the CI to propagate...

Copy link
Collaborator

@themarpe themarpe left a comment

Choose a reason for hiding this comment

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

CC: @moratom please integrate, thanks

@moratom moratom merged commit 2b517e1 into luxonis:master Jul 31, 2024
14 checks passed
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.

3 participants