-
-
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
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
| 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
|
can you try this diff Guido? diff --git a/Tools/pixi-packages/tsan-free-threading/recipe.yaml b/Tools/pixi-packages/tsan-free-threading/recipe.yaml
index dfae06ad7a5..3417e8b92b1 100644
--- a/Tools/pixi-packages/tsan-free-threading/recipe.yaml
+++ b/Tools/pixi-packages/tsan-free-threading/recipe.yaml
@@ -18,4 +18,6 @@ build:
env:
PYTHON_VARIANT: "tsan-free-threading"
+ python:
+ site_packages_path: "lib/python3.13t/site-packages"
# derived from https://github.com/conda-forge/python-feedstock/blob/main/recipe/meta.yaml |
Maybe I did something wrong, but I attempted to apply this patch over at ngoldbaum@39df6fe and then updated numpy at ngoldbaum/numpy@d28bebc and it doesn't seem to work, unfortunately, at least on my local setup. CI will test it when a runner picks it up: https://github.com/numpy/numpy/actions/runs/20754129965/job/59591290742?pr=30510 |
|
It looks like the option in the recipe isn't having any effect: |
|
I tried it locally and it at least works with Tools/pixi-packages/tsan-free-threading on 🎋 tsan [🔨?] via 🧚 v0.62.2
❯ pixi global install -e pyt --path ./python-3.15-h60d57d3_0.conda
└── pyt (installed)
├─ dependencies: python 3.15
└─ exposes: idle3, idle3.15, pip3, pip3.15, pydoc3, pydoc3.15, python, python3, python3-config, python3.15, python3.15-config, python3.15t, python3.15t-config
Tools/pixi-packages/tsan-free-threading on 🎋 tsan [🔨?] via 🧚 v0.62.2 took 2s
❯ pixi global install -e pyt cowpy
└── pyt (installed)
├─ dependencies: python 3.15, cowpy 1.1.5
└─ exposes: idle3, idle3.15, pip3, pip3.15, pydoc3, pydoc3.15, python, python3, python3-config, python3.15, python3.15-config, python3.15t, python3.15t-config, cowpy
Tools/pixi-packages/tsan-free-threading on 🎋 tsan [🔨?] via 🧚 v0.62.2
❯ ls -s ~/.pixi/envs/pyt/lib/python3.13t/site-packages/
╭───┬───────────────────────┬──────┬───────┬───────────────╮
│ # │ name │ type │ size │ modified │
├───┼───────────────────────┼──────┼───────┼───────────────┤
│ 0 │ cowpy │ dir │ 128 B │ 2 minutes ago │
│ 1 │ cowpy-1.1.5.dist-info │ dir │ 320 B │ 2 minutes ago │
╰───┴───────────────────────┴──────┴───────┴───────────────╯ |
|
ah probably that only works for |
That workaround is for older conda versions not handling noarch packages properly. |
|
Hmm yeah on second thought meson-python should be in the right place, let me try to reproduce Nathan's failure |
doh should have checked the CI log earlier, it is working! Just worked locally for me too |
Oh hey, it worked! That's so strange - I wonder what's different on my setup. It looks like pixi is up-to-date... |
|
if in doubt try |
Nope, doesn't work. Here's the build log after The only difference I see compared with the working build on CI is that my build used uv 0.9.22 but the CI build used 0.9.21. |
This ended up coming down to there being a |
- Remove clang-19 since we are not building the experimental jit - Remove ld_impl because we are not hacking the makefiles - Add site_packages_path - Make recipe.yaml identical with just the variant name difference - Add a generate.sh to generate recipe.yaml from default variant - Add a runtime requirement of libsanitizer for gcc
|
looks great, prefix-dev/pixi-build-backends#532 should get us down to a single |
|
Are symbolic links allowed? (Especially since these files end up in the python source distribution, it might be bad because of windows). If not we might have to duplicate the files. |
lucascolley
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.
could we update the README to point to the new blockers for using a single manifest and recipe per #142872 (comment) ?
Follow-up to #142469
Add pixi infrastructure for
free-threading: compiles with--disable-giltsan-free-threading: compiles with--disable-gil --with-thread-sanitizerCurrently,
free-threadingseems to work well on Linux.tsancrashes on build: [EDIT read comment below for solution]@kumaraditya303 @lucascolley any idea what's wrong here?