Add vegafusion-python-embed package #18081

jonmmease · 2022-02-17T15:45:42Z

This PR adds a recipe for the vegafusion-python-embed package. Project hompage is at https://vegafusion.io/.

This is a native Python package that wraps Rust logic using PyO3, compiled with the maturin build tool. The conda recipe pulls the official source from PyPI and builds it using maturin. This source distribution contains the project's own LICENSE file. A Thirdparty.yml file containing the licenses for third-party Rust dependencies (which are statically linked during the build process) is generated using cargo-bundle-licenses during the build processes.

For context, this package will rarely be used directly and after it is published two additional pure-python packages that depend on this one will follow (vegafusion and vegafusion-jupyter).

I'll ping once CI is passing. Thanks!

Checklist

Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
License file is packaged (see here for an example).
Source is from official source.
Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
If static libraries are linked in, the license of the static library is packaged.
Build number is 0.
A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
When in trouble, please check our knowledge base documentation before pinging a team.

conda-forge-linter · 2022-02-17T15:46:04Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/vegafusion-python-embed) and found it was in an excellent condition.

jonmmease · 2022-02-17T17:45:27Z

CI passing and artifact packages look good from my end.

Ready for review @conda-forge/help-python, @conda-forge/help-python-c.

Thanks!!

dhirschfeld · 2022-02-17T23:03:46Z

I'm excited to see this package available from CF! 🎉

@conda-forge/staged-recipes and @conda-forge/rust might be good to ping too.

… crate

dhirschfeld · 2022-02-18T12:37:34Z

The pertinent error seems to be:

error: invalid `--cfg` argument: `--target` (expected `key` or `key="value"`)

From:

...
+HOST=x86_64-conda-linux-gnu
+LDFLAGS=-Wl,-O2 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,--disable-new-dtags -Wl,--gc-sections -Wl,--allow-shlib-undefined -Wl,-rpath,$PREFIX/lib -Wl,-rpath-link,$PREFIX/lib -L$PREFIX/lib
Error: `cargo metadata` exited with an error: error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `rustc - --crate-name ___ --print=file-names --cfg --target x86_64-unknown-linux-gnu --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=cfg` (exit status: 1)
  --- stderr
  error: invalid `--cfg` argument: `--target` (expected `key` or `key="value"`)

jonmmease · 2022-02-18T17:31:12Z

Thanks @dhirschfeld. It looks like conda didn't like the space in the environment variable value. I think replacing it with an equal sign will work...

jonmmease · 2022-02-18T19:46:46Z

Ok, build is green again

bollwyvl · 2022-02-19T14:07:04Z

recipes/vegafusion-python-embed/meta.yaml

+    - python
+    - pip
+    - maturin
+    - cargo-bundle-licenses


cargo-bundle-licenses appears to usually go in build

works for me!

wolfv · 2022-02-21T08:47:47Z

recipes/vegafusion-python-embed/meta.yaml

+  number: 0
+  script: |
+    export RUSTFLAGS="--cfg unsound_local_offset"     # [not win]
+    set RUSTFLAGS=--cfg unsound_local_offset          # [win]


I think the "safe way" is to put a quotes around the entire env var plus value, like set "RUSTFLAGS=--cfg unsound_local_offset"

Thanks! Giving that a try now

wolfv · 2022-02-21T08:49:26Z

recipes/vegafusion-python-embed/meta.yaml

@@ -0,0 +1,57 @@
+{% set name = "vegafusion-python-embed" %}
+{% set name_under = "vegafusion_python_embed" %}
+{% set version = "0.0.4_rc.1" %}


is this an RC?

Thanks for the review @wolfv! I agree this should build from an actual release if at all possible.

Yeah, this is an RC because the current stable version doesn't have sdist bundles published. If this approach looks good otherwise, I can publish a new version.

I think if you're ready to release 0.0.4 proper then this recipe is in good shape to be merged.

Ofc, it's always possible further 👀 will pick up some more issues but it's easier for others to review when it's ready to merge (otherwise it would need a further review when it was ready)

CI running on version 0.0.4

jonmmease · 2022-02-25T16:00:19Z

Thanks for the reviews. Now built against version 0.0.4 final

jonmmease · 2022-02-28T14:30:15Z

friendly ping @wolfv @bollwyvl Is there anything else you'd like to see here? Thanks again for taking a look!

bollwyvl · 2022-03-01T16:22:55Z

Seems alright to me: i can't merge, though!

dhirschfeld · 2022-03-01T20:21:59Z

Seems alright to me: i can't merge, though!

Ping @conda-forge/staged-recipes!

tdegeus

Not a member of the core, but I saw your message on gitter.
Mostly thing looks good, though:

I would consider a different name, that keeps together the name of the package. E.g. vegafusion_embed-python, python-vegafusion_embed, or python-vegafusion-embed
I'm not sure that run_test.py is actually evaluated. But in any case I would avoid it all-together as indicated

tdegeus · 2022-03-05T14:51:49Z

recipes/vegafusion-python-embed/meta.yaml

+  commands:
+    - pip check


Suggested change

commands:

- pip check

commands:

- pip check

- python -c "import vegafusion_embed; vegafusion_embed.PyTaskGraphRuntime(4)"

Add initial vegafusion-python-embed recipe

07225e2

jonmmease mentioned this pull request Feb 17, 2022

[FeatureRequest] conda Python package vega/vegafusion#73

Closed

3 tasks

jonmmease changed the title ~~WIP: Add vegafusion-python-embed package~~ Add vegafusion-python-embed package Feb 17, 2022

Fix license identifier (It's AGPLv3 or later)

5067f40

Add RUSTFLAGS="--cfg unsound_local_offset" env var needed for time.rs…

255d7e1

… crate

Replace space with '=' to help conda build

bc5b6bd

Try no quotes around value on Windows

e01f791

bollwyvl reviewed Feb 19, 2022

View reviewed changes

Move cargo-bundle-licenses to build dependencies section

6cfde04

wolfv reviewed Feb 21, 2022

View reviewed changes

jonmmease and others added 3 commits February 21, 2022 08:29

Move quotes for windows environment variable

784e7ec

Update to 0.0.4 final

8469140

fix hash

fb7f493

tdegeus reviewed Mar 5, 2022

View reviewed changes

chrisburr merged commit 665af07 into conda-forge:main Mar 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add vegafusion-python-embed package #18081

Add vegafusion-python-embed package #18081

jonmmease commented Feb 17, 2022

conda-forge-linter commented Feb 17, 2022

jonmmease commented Feb 17, 2022

dhirschfeld commented Feb 17, 2022

dhirschfeld commented Feb 18, 2022 •

edited

jonmmease commented Feb 18, 2022

jonmmease commented Feb 18, 2022

bollwyvl Feb 19, 2022

jonmmease Feb 19, 2022

wolfv Feb 21, 2022

jonmmease Feb 21, 2022

wolfv Feb 21, 2022

dhirschfeld Feb 21, 2022

jonmmease Feb 21, 2022

dhirschfeld Feb 25, 2022

jonmmease Feb 25, 2022 •

edited

jonmmease commented Feb 25, 2022

jonmmease commented Feb 28, 2022

bollwyvl commented Mar 1, 2022

dhirschfeld commented Mar 1, 2022

tdegeus left a comment

tdegeus Mar 5, 2022

Add vegafusion-python-embed package #18081

Add vegafusion-python-embed package #18081

Conversation

jonmmease commented Feb 17, 2022

I'll ping once CI is passing. Thanks!

conda-forge-linter commented Feb 17, 2022

jonmmease commented Feb 17, 2022

dhirschfeld commented Feb 17, 2022

dhirschfeld commented Feb 18, 2022 • edited

jonmmease commented Feb 18, 2022

jonmmease commented Feb 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonmmease Feb 25, 2022 • edited

Choose a reason for hiding this comment

jonmmease commented Feb 25, 2022

jonmmease commented Feb 28, 2022

bollwyvl commented Mar 1, 2022

dhirschfeld commented Mar 1, 2022

tdegeus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhirschfeld commented Feb 18, 2022 •

edited

jonmmease Feb 25, 2022 •

edited