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
fix dotnet build cleanup #1678
fix dotnet build cleanup #1678
Conversation
8e9ae98
to
8bfa650
Compare
pre_commit/languages/dotnet.py
Outdated
if os.path.isfile(path): | ||
os.remove(path) | ||
elif f not in to_keep: | ||
rmtree(path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the problem with this is it will break when introducing language_version
for dotnet
as that allows multiple builds in a single clone
is there a way we can leverage git clean
to do this for us?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point -- I had hoped we could blitz the entire git directory after the tool install.
I'll look for a git clean
er solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's also the potential that a script
hook is defined in the same repo as well
8bfa650
to
76c5950
Compare
pre_commit/languages/dotnet.py
Outdated
prefix, | ||
( | ||
'git', 'clean', '-fxd', | ||
'-e', helpers.environment_dir(ENVIRONMENT_DIR, version), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably be f'{ENVIRONMENT_DIR}-*'
pre_commit/languages/dotnet.py
Outdated
helpers.run_setup_cmd( | ||
prefix, | ||
( | ||
'git', 'clean', '-fxd', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've actually got a video coming up soon that indicates this should be -ffxd
:)
76c5950
to
2b12203
Compare
2b12203
to
aa80234
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From testing, I found that the
bin
andobj
directories may be elsewhere depending on the particular hook project layout. I'd also like to cleanup the build directory properly, since we only need thedotnetenv-default
dir after we install the tool.This patch should solve both cases.