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

Preliminary support for the base component on Windows. #44

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

ScottTodd
Copy link
Member

Progress on #36.

Together with some other pending PRs, this gets me enough to

  1. Fetch sources
  2. Configure with CMake
  3. Build without errors

Not much is actually getting built, since this disables nearly all projects on Windows, but this is enough to start running CI builds and working on components incrementally.

@ScottTodd ScottTodd marked this pull request as ready for review February 1, 2025 00:23
CMakeLists.txt Outdated Show resolved Hide resolved
Comment on lines +65 to 68
if(NOT WIN32) # TODO(#36): Enable on Windows and/or make subproject inclusion generally optional

therock_cmake_subproject_declare(rocprofiler-register
EXTERNAL_SOURCE_DIR "rocprofiler-register"
Copy link
Member Author

Choose a reason for hiding this comment

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

Argh, and this also won't work due to a dep here:

therock_cmake_subproject_declare(ROCR-Runtime
EXTERNAL_SOURCE_DIR "ROCR-Runtime"
BACKGROUND_BUILD
CMAKE_ARGS
"-DBUILD_SHARED_LIBS=ON"
BUILD_DEPS
amd-llvm
RUNTIME_DEPS
rocprofiler-register

I had this "working" on Friday with my build directory, but now seeing issues with a fresh build.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah... That's a non optional dep to a few things

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I'm going in circles here. This is actually fine, as long as I have 'core' also excluded, which I do:

if(NOT WIN32)
  add_subdirectory(compiler)
  add_subdirectory(core)

Copy link
Member Author

Choose a reason for hiding this comment

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

Picking this back up with a fresh set of eyes and some hacks to get a CI runner past the symlink issues.

The excludes that I have here are sufficient to build, but they aren't a comfortable solution. I'd like to proceed with this approach for now then see about making rocprofiler-register (and other projects) compile successfully but no-op in some/all of its functionality as needed.

@ScottTodd ScottTodd marked this pull request as draft February 3, 2025 22:05
@ScottTodd ScottTodd marked this pull request as ready for review February 6, 2025 20:42
Copy link
Contributor

@stellaraccident stellaraccident left a comment

Choose a reason for hiding this comment

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

I think I'd like to come back through and add top level options for some of these things anyway. And can then cmake_dependent them if still useful once you get a little further.

fine like this for now

@ScottTodd ScottTodd merged commit f7005f7 into nod-ai:main Feb 7, 2025
1 check passed
@ScottTodd ScottTodd deleted the windows-bootstrap branch February 7, 2025 00:24
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