Skip to content

Use system/vcpkg spdlog when available otherwise fallback to fetchcontent#3139

Merged
9prady9 merged 2 commits into
arrayfire:masterfrom
9prady9:spdlog
Jul 6, 2021
Merged

Use system/vcpkg spdlog when available otherwise fallback to fetchcontent#3139
9prady9 merged 2 commits into
arrayfire:masterfrom
9prady9:spdlog

Conversation

@9prady9
Copy link
Copy Markdown
Member

@9prady9 9prady9 commented May 25, 2021 •
edited
Loading

Description

This change enables using system/vcpkg spdlog when available otherwise fallback to fetchcontent workflow. I have also udpated the spdlog version to 1.8.5 from the archaic 1.0.0 version. Fortunately, there were no issues like we encountered with fix releases during the version update of spdlog.

Apart from this, I have corrected the extern arrayfire deps download location; This is just a folder organization change, didn't fix any build issue.

Changes to Users

None

Checklist

  • Rebased on latest master
  • Code compiles
  • Tests pass
  • [ ] Functions added to unified API
  • [ ] Functions documented

@9prady9 9prady9 force-pushed the spdlog branch 2 times, most recently from 17f69f6 to 548c4da Compare May 26, 2021 07:04
@9prady9 9prady9 requested a review from umar456 May 26, 2021 07:53
@9prady9 9prady9 mentioned this pull request Jun 9, 2021
Merged
3 tasks
@9prady9 9prady9 added this to the 3.8.1 milestone Jun 21, 2021
@9prady9 9prady9 force-pushed the spdlog branch 3 times, most recently from 9bbd315 to dd52b19 Compare June 24, 2021 04:04
@9prady9
Copy link
Copy Markdown
Member Author

9prady9 commented Jul 3, 2021

Turns out the issue is with the dependency of spdlog, fmt whose latest version on vcpkg has compilation issues. I am working on a workaround. Will update the PR once it is ready

@9prady9
Copy link
Copy Markdown
Member Author

9prady9 commented Jul 3, 2021

the work around seemed to work on local windows setup, I wonder why it failed on CI after the change - I will look into the windows problem.

@9prady9
Copy link
Copy Markdown
Member Author

9prady9 commented Jul 5, 2021 •
edited
Loading

the work around seemed to work on local windows setup, I wonder why it failed on CI after the change - I will look into the windows problem.

I think this is due to a100 system being blocked by me for running another benchmark. I will shut that down now that I don't need it further. Will post windows update soon.

Update: Was able to reproduce this on Windows local build. Looking into it.

Update2: Bummer, I missed the spdlog include directory for nvcc compiled object files (different from afcuda target).

@9prady9 9prady9 force-pushed the spdlog branch 2 times, most recently from eb1cb50 to ad55a26 Compare July 5, 2021 15:48
umar456
umar456 reviewed Jul 6, 2021
View reviewed changes
Comment thread CMakeLists.txt
pradeep added 2 commits July 6, 2021 09:26
Correct extern arrayfire deps download location
362e025 
Although the build isn't broken, since forge project setup runs before
arrayfire fetch content variables are set, fetch-content-variables that
doesn't have suffixes are set by forge project specific settings. This
change fixes that.
Use system/vcpkg spdlog if not fallback to fetchcontent
a1d79d9
@9prady9 9prady9 merged commit 955152b into arrayfire:master Jul 6, 2021
@9prady9 9prady9 deleted the spdlog branch July 6, 2021 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@umar456 umar456 umar456 approved these changes

Assignees

No one assigned

Labels

build improvement

Projects

None yet

Milestone

3.8.1

Development

Successfully merging this pull request may close these issues.

2 participants

@9prady9 @umar456