tkittel
commented
1 month ago There are a lot more steps involved actually, but I have started working on it. I will be using this issue to collect random items that needs consideration.
- Need Windows workflow in the CI.
- The CMake code should be rewritten to work with multi-cfg generators like VisualStudio. Fortunately, Ninja-Multicfg can be used on unix as well, to test this.
- Need to add CTest-based testing, and need to port as many as possible of the simplebuild-based unit tests to that (to be included in the ncrystal repo where possible for license/size reasons). This is needed since simplebuild does not support Windows, and we need to support windows.
- In connection with the two above items, we should make it easier to develop NCrystal code directly out of the ncrystal repo (i.e. without the ncrystaldev simplebuild repo). One item is to look carefully at the CMake build area and ensure we have as few as possible copies of files, but include them directly from the src where possible.
- Must migrate all cmdline scripts to python modules, so we can use the entry-points magic of pip to deal with the .exe/.bat wrappers needed.
ncrystal-configwill need special consideration (would be nice if it could also be available as a python module... but would also be nice if we one day could split the c++/lib part of NCrystal into its own package. - We should add new unit tests addressing windows specific issues such as encodings in input/output, end of line chars, etc.
- Need to provide conda-forge and PyPI packages, but should also engage with local Windows users to ensure their way of using it will be OK.
- Must have CI guard against anyone committing a file the mctools/ncrystal which contains DOS line endings (NCrystal will be able to handle both, but we should keep it clean).
- By default the login in
ncapi.hwill setnc_cplusplusto C++14. But we should be sure to a have a windows test which tries to enable flags to use C++17, to cover more code paths. Equally, we need a debug build, and build with different options (ncrystal namespace, embedded data, etc.). - We should automate catch_fpe() and enable it to as many tests as possible, including python tests. It would also be great if it could work on windows.
- Using
-DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=ONright now to make tests with private headers work on windows. We need to make this automatic, but prevent it from being used to actually produce pip/conda packages (unless we want them to support advanced plugins?). We might also need to makeNCRYSTAL_APIa noop in that case, to get rid of warnings about duplicate dllexport. - We should double-check all usage of pathlib .read_text/.write_text since the default encoding is utf8 on unix but NOT on windows. In many cases we might have to specify an encoding=utf8 argument (e.g. if reading an .ncmat file)
- Remember that all CLI tools should use the new
from ._common import create_ArgumentParser. They also need a way to provide the argparser for sphinx. - To catch issues affecting windows on unix, we should make sure CI fails on unix if we forget an NCRYSTAL_API call on Windows. We should use the new test/modules to test the plugin mechanism.
- We need to exercise Ninja Multicfg generator on linux, to catch some VisualStudio generator issues.
- We should consider re-encoding windows strings into utf8 at the point of entry into NCrystal (for instance inside ncgetcwd we query GetCurrentDirectory, which we should encode as utf8 before returning - but also paths in cfg-strings might be an issue).
- We should make sure that we exercise a debug build (also for basictest.yml!)
- We should take the opportunity to increase lowest py version to 3.8, lowest cmake (to what?), rename PCBragg and PowderBragg, rename the ncrystal_core dir, etc.
- We should make the test compilations respect build_strict flags as well.
- should we migrate the exceptions as well, to only have NCLogicError and NCRuntimeError?
- Perhaps solve #152 and remove some of the setup.sh / setup.py / etc. hacks?
It would be nice to support windows. A reasonable first step would be to try to get the ncrystal package on conda-forge working on windows.