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

doc: Add workaround for Cythonized code and pytest #293069

Merged
merged 1 commit into from
Mar 20, 2024
Merged

doc: Add workaround for Cythonized code and pytest #293069

merged 1 commit into from
Mar 20, 2024

Conversation

sarahec
Copy link
Contributor

@sarahec sarahec commented Mar 3, 2024

Cython is a Python compiler that emits native .so modules. By default, python derivations run tests in the wrong directory to see these modules and tests fail.

Issue #255262 documents the root cause and solution for this problem.

This PR adds a description of the problem and the most common solution to the test troubleshooting list.

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@sarahec
Copy link
Contributor Author

sarahec commented Mar 3, 2024

@natsukium FYI. Let's get this knowledge into the documentation.

Copy link
Contributor Author

@sarahec sarahec left a comment

Choose a reason for hiding this comment

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

Fix typos, tighten last sentence.

@sarahec sarahec changed the title Add workaround for Cythonized code and pytest doc: Add workaround for Cythonized code and pytest Mar 3, 2024
Cython is a Python compiler that emits native .so modules. By default, python derivations run tests in the wrong directory to see these modules and tests fail.

Issue #255262 documents the root cause and solution for this problem.

This PR adds a description of the problem and the most common solution to the test troubleshooting list.
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

pytestCheckHook has nothing to do with Cython

@@ -2016,6 +2016,10 @@ example of such a situation is when `py.test` is used.

* Tests that attempt to access `$HOME` can be fixed by using the following
work-around before running tests (e.g. `preCheck`): `export HOME=$(mktemp -d)`
* Compiling with Cython causes tests to fail with a `ModuleNotLoadedError`.
This can be fixed with two changes in the derivation: 1) replacing `pytest` with
`pytestCheckHook` and 2) adding a `preCheck` containing `cd $out` to run
Copy link
Member

Choose a reason for hiding this comment

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

Maybe cd / or something should be preferred because we really don't want to import anything that's not in $PYTHONPATH.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm basing this on #255262 and the discussion there as well as fixing two packages' tests with this technique. If there's a better suggestion, I'm all ears.

Copy link
Member

Choose a reason for hiding this comment

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

It's indeed important that whatever is installed takes precedence. In that sense, any folder but the source folder is good enough from that point of view. Then, comes the next, which @dotlambda mentions, we don't want to pick up anything more that we don't want either. Therefore, some folder where there is nothing of interest would be preferred.

Note it is also important to switch back directly again (assuming cd is used) for later phases/hooks.

@DanielSidhion
Copy link
Member

Hi folks, this PR has been open for a while but I don't know enough about the cython-related stuff to know whether this is mergeable in its current state or if it needs any changes. I'd like to help merge documentation PRs quicker as long as they're mergeable, can you help me decide whether this is the case here already?

@FRidh FRidh merged commit 2f7f71e into NixOS:master Mar 20, 2024
19 checks passed
@sarahec sarahec deleted the patch-1 branch March 28, 2024 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants