Repository navigation
[Fix] Fall back to ~/.cache/tilus when git is not installed - #166
Merged
Merged
Conversation
tilus.option computes the default cache directory at import time by running "git rev-parse" next to the package. A failing git command was already handled and sends the cache to ~/.cache/tilus, but when there is no git binary on PATH subprocess.run raises FileNotFoundError instead of CalledProcessError. That escaped the except clause, so "import tilus" failed outright in slim containers and other environments without git. Catch OSError together with CalledProcessError so a missing git takes the same fallback as a failing one, and add a test that covers both cases. Signed-off-by: Dmitry Voropaev <dy.voropaev@gmail.com>
When tilus is installed into a site-packages directory that happens to live inside another git work tree (e.g. a virtualenv inside a project repo), 'git rev-parse --is-inside-work-tree' succeeds and the default cache dir ended up at site-packages/../.cache. Compare 'git rev-parse --show-toplevel' with the tilus repository root instead, and fall back to ~/.cache/tilus otherwise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
|
/ok to test 2e3ba71 |
Member
|
thanks @v0ropaev ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_get_default_cache_dir()inpython/tilus/option.pyrunsgit rev-parse --is-inside-work-treewhen the module is imported, and falls back to~/.cache/tilusonsubprocess.CalledProcessError. If git is not installed at all,subprocess.runraisesFileNotFoundErrorinstead, which is not caught. Soimport tilusfails in a container or environment that has no git binary. With PATH pointed at a nonexistent directory I getThe except clause now catches
OSErroras well, so a missing git binary takes the same fallback as a failing git command.OSErrorrather than onlyFileNotFoundErroralso covers a git that exists but cannot be executed, which should not break the import either. I also addedtests/test_option.py, which patchessubprocess.runto raise either error and checks that the result is~/.cache/tilusunder a patched home directory.I tested on macOS without a GPU, so
cuda.bindingswas replaced by a stub module on PYTHONPATH outside the repo. With PATH set to a nonexistent directory,python -c 'import tilus'raises the error above on main. With this change the import succeeds andget_option("cache_dir")returns~/.cache/tilus. The new test fails on the FileNotFoundError case without the fix and passes with it.pytest tests/test_option.py tests/test_multiprocess.py tests/drivers/test_cache.py tests/test_compiler_fingerprint.pygives 21 passed, andpre-commit runon the changed files passes.