lint: fix custom mypy cache dir setting #28184
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
test/lint/lint-python.py
Outdated
There was a problem hiding this comment.
If it is not needed, it may be better to remove it.
There was a problem hiding this comment.
This was added as part of #18210 but I didn't see an explanation given there. I think it is better to specify a location explicitly since the default is using the current directory which may be leading to unexpected results if this script is called from different contexts. But if we don't care about something like this, it may be removed.
test/lint/lint-python.py
Outdated
There was a problem hiding this comment.
This will write to /test/ if a user is running this script:
# ./test/lint/lint-python.py
Success: no issues found in 269 source files
# ls /test/.mypy_cache/
3.11 CACHEDIR.TAG
There was a problem hiding this comment.
That was the previous behavior. There is even an explicit line for this in our .gitignore. What would you like to do instead? Move it into /test/cache/?
There was a problem hiding this comment.
No, I mean /test/, not ./test/.
With /test being next to /bin, etc. I don't think .gitignore will work on the literal root of your filesystem. .gitignore should be limited to the folder it is located in.
There was a problem hiding this comment.
I see, I simply misread. I changed this to get the base path relative to file as a fallback. We could also simply use . or fall back to not setting a cache_dir at all but this seemed the most robust option to me.
There was a problem hiding this comment.
I see, I simply misread. I changed this to get the base path relative to file as a fallback. We could also simply use
.or fall back to not setting acache_dirat all but this seemed the most robust option to me.
Wouldn't the default option result in placing the cache dir in the root, regardless of where the script is being run from?
|
Are you still working on this? If not, could close, so that someone else can pick it up? |
fjahr
force-pushed
the
202307-mypy-cache
branch
from
d001781 to
55219b3
Compare
fjahr
force-pushed
the
202307-mypy-cache
branch
from
55219b3 to
d520273
Compare
fjahr
force-pushed
the
202307-mypy-cache
branch
from
d520273 to
b5825de
Compare
|
Addressed feedback by @MarcoFalke and rebased |
fjahr
force-pushed
the
202307-mypy-cache
branch
from
b5825de to
b37e70e
Compare
fjahr
force-pushed
the
202307-mypy-cache
branch
from
b37e70e to
f904777
Compare
|
lgtm ACK f904777 |
f904777 lint: fix custom mypy cache dir setting (Fabian Jahr) Pull request description: fixes bitcoin#28183 The custom cache dir for `mypy` can only be set via an environment variable, setting the `MYPY_CACHE_DIR` variable in the program is not sufficient. This error was introduced while translating the shell script to python. See also the mypy documentation: https://mypy.readthedocs.io/en/stable/config_file.html#confval-cache_dir ACKs for top commit: MarcoFalke: lgtm ACK f904777 Tree-SHA512: 7e8fb0cd06688129bd46d1afb8647262eb53d0f60b1ef6f288fedaa122d906fb62c9855e8bb0d6c6297d41a87a47d3cec7a00df55a7d033947937dfe23d07ba7
5 participants
fixes #28183
The custom cache dir for
mypycan only be set via an environment variable, setting theMYPY_CACHE_DIRvariable in the program is not sufficient. This error was introduced while translating the shell script to python.See also the mypy documentation: https://mypy.readthedocs.io/en/stable/config_file.html#confval-cache_dir