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

CMake Refactor #72

Merged
merged 89 commits into from
Mar 22, 2024
Merged

CMake Refactor #72

merged 89 commits into from
Mar 22, 2024

Conversation

Curve
Copy link
Member

@Curve Curve commented Mar 14, 2024

This Pull Request modernizes CMake (resolves #56):

  • Use CPM / PackageProject

  • Update Include Structure

    • Don't artificially flatten
    • Use q-char-sequences where appropriate
  • Copy required shared libraries when required / Setup proper path automatically when required

  • Modernize Python Bindings (integration with gdsfactory and other open source python based EDA tools #65)

    • Use scikit-build-core
    • Re-use the shared libraries contained in the VTK-Python package
      • On Linux (using RPATH) [✅ Tested]
      • On MacOS (using RPATH) [✅ Tested]
      • On Windows (os.add_dll_directory) [⏳ Via viennaps.libs folder, awaiting upstream changes]
  • Add library switch to ease location of installed dependencies (ViennaLS, ViennaRay, ...)

@Curve Curve changed the title Cmake Refactor (closes #56) Cmake Refactor Mar 14, 2024
@Curve Curve changed the title Cmake Refactor CMake Refactor Mar 14, 2024
@Curve Curve marked this pull request as ready for review March 14, 2024 23:35
@Curve
Copy link
Member Author

Curve commented Mar 14, 2024

Note to self: I still have to adjust the workflows and the app

std::vector<cellType &> getNeighbors(const cellType &cell) {
std::vector<cellType &> neighbors;
auto cellSet = domain->getCellSet();
// What happened here?
Copy link
Member Author

Choose a reason for hiding this comment

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

@tobre1 Was this some sort of merge conflict on your side?

@Curve
Copy link
Member Author

Curve commented Mar 17, 2024

I've evaluated some possible solutions but most are way too hacky to be deemed viable - I have thus raised an issue on VTKs side: https://gitlab.kitware.com/vtk/vtk/-/issues/19279.

For now I'm going to add a viennaps.libs folder on those targets that require it (currently only windows) and copy the VTK libraries there for the time being.

@Curve
Copy link
Member Author

Curve commented Mar 18, 2024

The VTK libraries are now installed in the viennaps.libs folder only on windows for the time being - In case the VTK issue is resolved we could remove the libs folder on windows as well :)

@tobre1
Copy link
Member

tobre1 commented Mar 19, 2024

When building the examples, the binaries are now placed in build/examples-bin/. This change should also be addressed in the README Basic Examples - Building section.

@tobre1
Copy link
Member

tobre1 commented Mar 19, 2024

When local ViennaLS/ViennaRay packages are used (specified in VIENNAPS_LOOKUP_DIRS), OpenMP is not added to the dependencies when building the examples. I get the following error:

[cmake] CMake Error at examples/GDSReader/CMakeLists.txt:3 (add_executable):
[cmake]   Target "GDSReader" links to target "OpenMP::OpenMP_CXX" but the target was
[cmake]   not found.  Perhaps a find_package() call is missing for an IMPORTED
[cmake]   target, or an ALIAS target is missing?

@Curve
Copy link
Member Author

Curve commented Mar 19, 2024

When building the examples, the binaries are now placed in build/examples-bin/. This change should also be addressed in the README Basic Examples - Building section.

When local ViennaLS/ViennaRay packages are used (specified in VIENNAPS_LOOKUP_DIRS), OpenMP is not added to the dependencies when building the examples. I get the following error:

[cmake] CMake Error at examples/GDSReader/CMakeLists.txt:3 (add_executable):
[cmake]   Target "GDSReader" links to target "OpenMP::OpenMP_CXX" but the target was
[cmake]   not found.  Perhaps a find_package() call is missing for an IMPORTED
[cmake]   target, or an ALIAS target is missing?

Thanks for the thorough testing!

I've updated the README and pushed a fix for ViennaRay (regarding OpenMP) here: ViennaTools/ViennaRay#48

Please try to install ViennaRay locally again with commit 6f2d980

@tobre1 tobre1 merged commit 1af21bd into master Mar 22, 2024
24 checks passed
@tobre1 tobre1 deleted the cmake-refactor branch March 22, 2024 10:27
@Curve
Copy link
Member Author

Curve commented Mar 23, 2024

I've evaluated some possible solutions but most are way too hacky to be deemed viable - I have thus raised an issue on VTKs side: https://gitlab.kitware.com/vtk/vtk/-/issues/19279.

For now I'm going to add a viennaps.libs folder on those targets that require it (currently only windows) and copy the VTK libraries there for the time being.

VTK is working on a new way to make this possible, however it all seems to be highly work in progress right now

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.

Build system refactoring
2 participants