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

Python package metadata contains local paths #335

Open
KOLANICH opened this issue May 22, 2023 · 3 comments
Open

Python package metadata contains local paths #335

KOLANICH opened this issue May 22, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@KOLANICH
Copy link

Describe the bug

Requires-Dist: symforce-sym @ file://localhost/PATH_TO_symforce_CLONED_REPO/gen/python

To Reproduce
Steps to reproduce the behavior, e.g.:

  1. python3 -m build -nwx

Expected behavior
No such things should be in wheels.

@KOLANICH KOLANICH added the bug Something isn't working label May 22, 2023
@aaron-skydio
Copy link
Member

We have the SYMFORCE_REWRITE_LOCAL_DEPENDENCIES environment variable to control this - when building wheels to distribute, the intention is to set that environment variable, and distribute wheels for symforce-sym and skymarshal separately. If that doesn't work for some reason we should fix that.

Agree this isn't ideal, the reason it's set up like this is so that pip install . from the SymForce directory "just works".

I'm curious if there's a specific issue this is causing, or if there's a specific proposal to change this in a way that both supports building standalone wheels for these things and one-line from-source installs

@KOLANICH
Copy link
Author

Thanks for the info and I'm sorry for the inconvenience of the trivial questions, I really haven't known that for building a wheel properly I need to set some environment variables. I close this issue for now, because I currently have no plans to rebuild the wheel (it takes a lot of time and a lot of memory, and some manual tweaks (like digging into files and disabling tests, since building them takes even more memory)) in order to test if setting the var solves the issue.

@aaron-skydio
Copy link
Member

We aren't supposed to be building tests, examples, or benchmarks when building wheels (benchmarks don't build by default, which is why they aren't explicitly disabled there, although it wouldn't be a bad idea to explicitly disable them also):

symforce/setup.py

Lines 114 to 119 in c3d2f3c

# NOTE(hayk): Don't build tests or examples through pip.
cmake_args += [
f"-DCMAKE_BUILD_TYPE={cfg}",
"-DSYMFORCE_BUILD_TESTS=OFF",
"-DSYMFORCE_BUILD_EXAMPLES=OFF",
]

If we are, that's a bug that we should also fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants