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

new example: ctypes #441

Merged
merged 5 commits into from
Nov 16, 2023
Merged

Conversation

liquidcarbon
Copy link
Contributor

Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR. I mentioned some small improvements.

examples/ctypes-factorial/pixi.toml Outdated Show resolved Hide resolved

[tasks]
compile = {cmd = "gcc -shared -o src/factorial.so -fPIC src/factorial.c"}
factorial = {cmd = "python src/factorial.py $1 $2", depends_on = "compile"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
factorial = {cmd = "python src/factorial.py $1 $2", depends_on = "compile"}
factorial = {cmd = "python src/factorial.py, depends_on = "compile"}

We're not yet supporting numbered arguments but everything you would add to pixi run factorial xxx would be send to that task so in this case you wouldn't need them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! It's worth calling out in README. Is there documentation I could reference? I wasn't able to find this in Pixi or Deno docs, so I just gave it a try, it worked, and I assumed that the $1 $2 was necessary

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added it to the new docs PR : bf0c0c0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I'll wrap it up later this week. Thanks for your input!

platforms = ["linux-64"]

[tasks]
compile = {cmd = "gcc -shared -o src/factorial.so -fPIC src/factorial.c"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try to include a start task in all our maybe it could be nice to add a factorial call but with some pre filled values. We do this to avoid user to have to read the README before they can test it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not remove the engine argument and make it print both runs in the same output. This makes it easy to get a quick overview.

platforms

Co-authored-by: Ruben Arts <ruben@prefix.dev>
@liquidcarbon
Copy link
Contributor Author

@ruben-arts would you mind dropping Windows from this one? I'd rather not get into it at this time.
Having trouble with the cpp example on Win as well.

@ruben-arts
Copy link
Contributor

@ruben-arts would you mind dropping Windows from this one? I'd rather not get into it at this time. Having trouble with the cpp example on Win as well.

He funnily enough, I just got a Windows machine for testing these type of pixi problems. The trick here is to run pixi add m2w64-gcc --platform win-64 and its better to replace gcc with c-compilers

This worked for me.

@liquidcarbon
Copy link
Contributor Author

I decided to keep the engine argument to showcase task dependencies and passing arguments between tasks and the underlying scripts. They play along really well!

No Mac handy to try to make it work there. I understand gcc would probably not work.
But I tested under Windows and Linux (WSL Ubuntu) and it's good enough for me. :)

@ruben-arts
Copy link
Contributor

Hey @wolfv or @tdejager could you test it on your Mac?

@wolfv
Copy link
Member

wolfv commented Nov 13, 2023

I got it to work by adding these two:

[target.osx-arm64.tasks]
compile = {cmd = "clang -shared -o src/factorial.so src/factorial.c"}

[target.osx-arm64.dependencies]
clang-16 = "*"

@liquidcarbon
Copy link
Contributor Author

liquidcarbon commented Nov 13, 2023

So should pixi.toml be

[dependencies]
python = "3.12.0.*"
loguru = "0.7.2.*"

[target.win-64.dependencies]
m2w64-gcc = "5.3.0.*"

[target.linux-64.dependencies]
gcc = "13.2.0.*"

[target.osx-64.dependencies]
clang-16 = "*"

[target.osx-arm64.dependencies]
clang-16 = "*"


[tasks]
compile = {cmd = "gcc -shared -o src/factorial.so src/factorial.c"}
factorial = { cmd = "python src/factorial.py -e python"}
start = { cmd = [
    "python",
    "src/factorial.py",
    "12345678",
    ";",
    "python",
    "src/factorial.py",
    "12345678",
    "-e",
    "python",
], depends_on = "compile" }

[target.osx.tasks]
compile = {cmd = "clang -shared -o src/factorial.so src/factorial.c"}

[target.osx-arm64.tasks]
compile = {cmd = "clang -shared -o src/factorial.so src/factorial.c"}

will pixi know to use tasks for Win and Linux and platform-specific tasks for MacOS?

@ruben-arts
Copy link
Contributor

Yes that would work @liquidcarbon

@ruben-arts
Copy link
Contributor

The lockfile is not uptodate 😄

@ruben-arts ruben-arts merged commit 04875d2 into prefix-dev:main Nov 16, 2023
@ruben-arts
Copy link
Contributor

Thank you @liquidcarbon another good example!!

@liquidcarbon liquidcarbon deleted the docs/examples-ctypes branch November 16, 2023 14:48
@liquidcarbon
Copy link
Contributor Author

Thanks for your help!

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.

3 participants