Conversation
Python instantiates processes differently across platforms. On Linux, processes are instantiated using "fork", whereas on Windows and macOS, processes are instantiated using "spawn". Prior to this commit, multiprocessing failed on Windows and macOS, due to the assumption that processes are always forked (and hence inherit resources from the parent process). The failure was related to the dynamic import of the configuration modules. See https://book-of-gehn.github.io/articles/2022/03/06/Multiprocessing-Spawn-of-Dynamically-Imported-Code.html. This commit re-writes multiprocessing such that processes are spawned on all platforms, which will be the default as of Python 3.14 anyway, see https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods.
Parse arguments (kind of test and path to InChI library) directly from the command line. Before this commit, those arguments were configured as TestConfig in dedicated files, which was less flexible and more complex.
fbaensch-beilstein
left a comment
fbaensch-beilstein
left a comment
There was a problem hiding this comment.
Overall looks good to me, would be awesome if we could integrate it for the next release
| shell: bash | ||
|
|
||
| - name: Write invariance summary | ||
| if: '!cancelled()' |
There was a problem hiding this comment.
I think Github prefers the ${{ }} syntax, although he encapsulation with ' ' works as well
There was a problem hiding this comment.
Same for the other if statement in this file
|
|
||
| }, | ||
| { | ||
| "os": "macos-14", |
There was a problem hiding this comment.
Please make sure, that this must run on M1/ARM runners, so the CMake must not contain any x86_64 specific flags
| on: | ||
| workflow_dispatch: | ||
| push: | ||
| # TODO: Remove branches trigger. |
There was a problem hiding this comment.
Reminder to do this TODO ;)
| RELEASE_DIR: inchi-${{ matrix.slug }}-${{ github.sha }} | ||
|
|
||
| permissions: | ||
| contents: write |
There was a problem hiding this comment.
Does the build_and_test block needs the permission to write?
| run: zip -r cmake_configurations.zip cmake_configurations | ||
|
|
||
| - name: Create release | ||
| # TODO: lift this guard to release job level |
There was a problem hiding this comment.
Reminder to do the TODO ;)
Lift the if statement up to for example line 122
| id: define_matrix | ||
| # The build matrix is used to interpolate commands and inputs throughout the remainder of the workflow. | ||
| # Since those interpolations are potentially security-sensitive, we verify the hash of the build matrix. | ||
| if: ${{ hashFiles('.github/workflows/build_matrix.json') == 'ae87ded543b131908cf5aed1502d0e39b2cb9374693598ef7f09c5c5a8be4e52' }} |
There was a problem hiding this comment.
Maybe add an else statement here that prints a dedicated error message here if the hash check fails
3 participants
This PR implements #1.
Have a look at the logs of the CI run on this PR to get an impression of how the release automation builds artifacts for the different platforms.