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

Initial Windows Support for ENDFtk #217

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

HunterBelanger
Copy link

This PR makes a few modifications so that ENDFtk can be compiled natively on Windows. For now, the changes have been quite minor:

  • Use development branch of NJOY tools to avoid problems with auto returns in the logger
  • Change two methods inside InterpolationBase to avoid MSVC compiler errors due to calling previously defined auto return methods
  • Change a CMake setting to avoid MSVC complaining about unknown compiler flags
  • Update two unit tests that were failing due to iterator invalidation

After making these changes, all the tests passed on my system (Windows 11 with MSVC 19.40). There are still, however, a few issues to address:

  • Windows and CMake were constantly complaining about file paths which are too long (almost exclusively in the unit tests). Because of this, some of the tests just wouldn't even be built. To fix this, I had to update my Windows registry to allow longer file names. This allowed those tests to be built, and the passed, but CMake and Windows is still complaining about paths that are too long and can't be tracked.
  • Initially, two unit tests were failing. After examination, it appeared that this was likely due to iterator invalidation when adding strings together in the test. Fixing this which allowed the tests to pass. I have not checked to see if this type of problem exists in other tests though, so this might be something that needs to be addressed elsewhere.
  • For now, I have just set the dependencies to pull the development branch of NJOY tools. I made a parallel PR there to fix the unknown compiler flag warnings in that package. At some point, we will need to set this dependency to a commit hash.

@whaeck I am now sure how you want to address these remaining problems (primarily the first one) ?

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.

1 participant