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

Flake broken after commit e48a60c - Change in binary file detection causes infinite hang #20

Open
MRocholl opened this issue Jan 9, 2025 · 3 comments

Comments

@MRocholl
Copy link

MRocholl commented Jan 9, 2025

Hello and thanks for this awesome flake!
The recent change in commit e48a60c appears to have broken the flake functionality. This commit modified the binary file detection logic from:

find "$(realpath "$VENV_PATH")" -type f -executable -exec sh -c "file -i '{}' | grep -qE 'x-(.*); charset=binary'" \; -print

to:

find "$(realpath "$VENV_PATH")" -type f -exec sh -c "file -i '{}' | grep -qE 'application/x-(executable|sharedlib); charset=binary'" \; -print

While this change was made to address issue #19 regarding -executable excluding too many files, it causes the flake to hang indefinitely when trying to use it.

Steps to reproduce:

  1. Use nix shell with the repository URL
  2. Run: nix shell github:GuillaumeDesforges/fix-python/e48a60c2d6a888ad2c5ddfcf36590c924f5df58a
  3. The command works normally.

Now:

  1. Use nix shell with the repository URL
  2. Run: nix shell github:GuillaumeDesforges/fix-python/
  3. Stuck indinitely at:
Automatically adding "patchelf" to PATH.
Searching for files to patch in .venv

This appears to be related to the change in binary file detection logic, possibly due to the removal of the -executable filter causing the search to process too many files or enter an infinite loop.

Im gonna stick to the older commit for now as it just works for me. But this should probably be investigated.

@GuillaumeDesforges
Copy link
Owner

Thank you very much for the thorough report!

Could you please share the packages in your venv (for instance with pip freeze)?

How did you create your venv? Did you use the stdlib venv module or a third party CLI?

@MRocholl
Copy link
Author

MRocholl commented Jan 10, 2025

The venv was created via poetry.
Ive exported the requirements via uv.
requirements.txt

Tested with a differnt repository, where the venv was created via uv. It worked. but took significantly longer. (only installed pandas). This might simply be a performance issue in that case. But for larger venvs, this might be almost prohibitively long.

@GuillaumeDesforges
Copy link
Owner

Thank you. That makes sense and I have noticed a slow down as well (though not as big as you have, probably for diverse reason).

This issue is definitely "a bug" that needs to be fixed.

An option is to have both mechanisms to find files that can be used, depending on the context. Maybe I should allow using the former mechanism with a --fast flag. Maybe I should revert to the former mechanism and add a flag --deep to use the new mechanism.

The best answer is probably to rewrite it in [insert low level language], which I'm very interested to do, but realistically don't have the time at the moment.

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

No branches or pull requests

2 participants