-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-143120: pixi builds for free-threading and TSAN #142872
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
base: main
Are you sure you want to change the base?
Conversation
| script: | ||
| file: ../build.sh | ||
| env: | ||
| PYTHON_VARIANT: "free-threading" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy-paste of asan/recipe.yaml, except this one line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy-paste of asan/pixi.toml
Tools/pixi-packages/tsan/recipe.yaml
Outdated
| script: | ||
| file: ../build.sh | ||
| env: | ||
| PYTHON_VARIANT: "tsan" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy-paste of asan/recipe.yaml, except this one line
no clue sorry I haven't seen this before |
|
The tsan crash is due to the default $ sudo sysctl vm.mmap_rnd_bits
vm.mmap_rnd_bits = 32 # too high
$ sudo sysctl vm.mmap_rnd_bits=28 # reduce it
vm.mmap_rnd_bits = 28
|
|
There's a lot of copy/paste here, which can make maintenance harder. Does pixi have some concept of code reuse or parametrisation? Maybe not the best example, but something like GitHub Actions' reusable workflows? |
|
@lucascolley is there anything we can do right now to reduce the duplication here? Or does that require fixes in pixi? |
|
As mentioned in the README already, this is blocked on prefix-dev/pixi#4599. |
ngoldbaum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened an issue which is needed to make this mergeable. I'm also playing with a NumPy recipe based on this.
Tools/pixi-packages/README.md
Outdated
| - `asan`: ASan-instrumented build with `PYTHON_ASAN=1` | ||
| - `free-threading` | ||
| - `asan`: ASan-instrumented build | ||
| - `tsan`: free-threading, TSan-instrumented build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably be called tsan-free-threading. There should probably also be an asan-free-threading. @lucascolley is there a way to avoid the combinatorial explosion of variants here? Ideally you'd be able to somehow combine these but I think that's probably not tenable at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the answer to this is no because of prefix-dev/pixi#4599
Barring the above, we could write a template + generation script and then commit the output to git? Very ugly if you ask me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I would vote for keeping things simple but verbose at the minute until there is a proper solution upstream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to tsan-free-threading
|
Ready for final review |
You can ignore it. Harmless warning. |
|
This is out of scope of this PR and can be addressed in a follow-up.
As the image is macOS 15, this feels like it should be fixable with an env variable? |
Co-authored-by: Lucas Colley <lucas.colley8@gmail.com>
|
@isuruf the libasan fix works in my hdf5 recipe, but not in the python recipe: Reverting for now. |
|
Your hdf5 recipe is still not using fd6ca6d. Can we please stop the reverting back and forth and debug why the correct commit from cpython is not being used? |
|
For the sake of test velocity and thoroughness, I've created https://github.com/crusaderky/cpython-pixi @isuruf the latest macos-15-intel changes let the python build complete; thank you. Pending
|
@hugovk would you be able to advise? |
@isuruf I appreciate your frustration. I'm frustrated too by this issue. I'm fairly sure that the last CI run I pointed to shows the effect of the libcompiler-rt changes - because I checked - but I'm fallible, so I'll do it again this time making extra sure not to muddle any evidence afterwards. For the sake of velocity and clarity I've moved my CI to https://github.com/crusaderky/cpython-pixi |
Let's not add symlinks to the repo, they're harder to use on Windows and make cross-platform support generally more complex. |
Follow-up to #142469
freethreading: compiles with--disable-giltsan-freethreading: compiles with--disable-gil --with-thread-sanitizerrecipe.yamlandpixi.tomlall identical symlinksmacos-15-intelgithub runners)Tested in CI on https://github.com/crusaderky/hdf5-pixi