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

Be careful about left-over files from git add --edit runs #107

Closed
wants to merge 1 commit into from

Conversation

dscho
Copy link
Member

@dscho dscho commented Jan 14, 2019

J Wyman reported almost a year ago (and I fixed this issue in Git for Windows around that time) that the .git/ADD_EDIT.patch file might still lie around at the beginning of git add --edit from previous runs, and if the new patch is smaller than the old one, the resulting diff is obviously corrupt.

This is yet another Git for Windows patch finally making it to the Git mailing list.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 14, 2019

Welcome to GitGitGadget

Hi @dscho, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that this Pull Request has a good description, as it will be used as cover letter.

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

  • the lines should not exceed 76 columns,
  • the first line should be like a header and typically start with a prefix like "tests:" or "commit:", and
  • the commit messages' body should be describing the "why?" of the change.
  • Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a PR comment of the form /allow <username>.

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions.

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox ("raw") file corresponding to the mail you want to reply to from the Git mailing list. If you use GMail, you can upload that raw mbox file via:

curl -g --user "<EMailAddress>:<Password>" --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

If there is already a .git/ADD_EDIT.patch file, we fail to truncate it
properly, which could result in very funny errors.

Of course, this file should not be left lying around. But at least in
one case, there was a stale copy, larger than the current diff. So the
result was a corrupt diff.

Let's just truncate the file when we write it and not worry about it too
much.

Reported by J Wyman.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member Author

dscho commented Jan 15, 2019

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 15, 2019

Submitted as pull.107.git.gitgitgadget@gmail.com

@dscho
Copy link
Member Author

dscho commented Jan 29, 2019

This actually already made it to next: fa6f225

@dscho dscho added duplicate This issue or pull request already exists next pu and removed duplicate This issue or pull request already exists labels Jan 29, 2019
@dscho
Copy link
Member Author

dscho commented Jan 29, 2019

And it got the name js/add-e-clear-patch-before-stating.

@dscho
Copy link
Member Author

dscho commented Jan 29, 2019

7fa92ba merged it into master

@dscho dscho closed this Jan 29, 2019
@dscho dscho deleted the add-e-truncate branch January 29, 2019 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant