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

Change clean command to only operate on non-encrypted files as recommended by git docs #179

Closed
wants to merge 1 commit into from

Conversation

jirutka
Copy link

@jirutka jirutka commented Jul 28, 2019

As recommended by gitattributes(5):

For best results, clean should not alter its output further if it is run twice ("clean->clean" should be equivalent to "clean"), and multiple smudge commands should not alter clean's output ("smudge->smudge->clean" should be equivalent to "clean").

I've extracted this change from #107.

Co-Authored-By: @shlomosh

As recommended by gitattributes(5):

> For best results, clean should not alter its output further if it is
> run twice ("clean->clean" should be equivalent to "clean"), and
> multiple smudge commands should not alter clean's output
> ("smudge->smudge->clean" should be equivalent to "clean").

I've extracted this change from AGWA#107.

Co-Authored-By: Shlomo Shachar <[email protected]>
@jirutka jirutka changed the title Change clean command to only operate on non-encrypted files Change clean command to only operate on non-encrypted files as recommended by git docs Jul 28, 2019
@AGWA
Copy link
Owner

AGWA commented Jul 28, 2019

Does this solve an actual problem? I understand that this is recommended by gitattributes(5), but that man page is thinking of filters that do things like strip carriage returns, which could hypothetically have already been done before committing. Under what circumstances could a file already be encrypted when being committed (that doesn't indicate a deeper problem that needs to be fixed)? I am very hesitant about adding a code path that could cause encryption to be bypassed, so I am seeking to really understand the motivation behind this. Thanks.

@jirutka
Copy link
Author

jirutka commented Jul 28, 2019

To be honest, I don’t know. I’m just refactoring #107 and this change is not related to the merge command, so I’ve extracted it into a separate PR.

@AGWA AGWA closed this Jul 29, 2020
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

Successfully merging this pull request may close these issues.

2 participants