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

Remove *.orig files from version control #768

Conversation

nymius
Copy link
Contributor

@nymius nymius commented Dec 13, 2024

Looking at the changes of last update to version 6.0 of libsecp256k1 I noticed the file extension of two new added files were .orig.
Considering the context, I assumed these files are the result of the execution of git mergetool, and were not intended to be included in the commit initially.
Trying to understand if they served another purpose I realized a third orig file was committed with the update of libsecp256k1 to version 4.0,
I checked for any references for these files in the codebase but didn't find any.
I also checked the libsecp256k1 to know if this was something committed in that repository but I confirm is not.

To keep the source code tree clean I removed these files, and also added a new .gitignore rule to avoid the inclusion of these files in the future.

By git mergetool docs:
> git mergetool creates *.orig backup files while resolving merges. These
> are safe to remove once a file has been merged and its git mergetool
> session has completed.

As there were no references found to these *.orig files in the codebase,
the assumption is that these files have been included accidentally by
executing some variation of `git add secp256k1-sys/depend/secp256k1/*`,
and they seem to not serve any use case, therefore they will be removed
from version control by this commit.
By git mergetool docs:
> git mergetool creates *.orig backup files while resolving merges. These
> are safe to remove once a file has been merged and its git mergetool
> session has completed.

These files don't serve any use case in the source version control, so
are going to be ignored from version control from this commit on.
@apoelstra
Copy link
Member

Thanks for checking so carefully!

These are part of the output of patch when used as part of secp256k1-sys/vendor-libsecp.sh. They don't serve any purpose except that they show up when people run the revendoring script. It might be worthwhile to modify the script itself to notice and delete these files (since we don't want people manually changing the output of the revendoring script) -- though just gitignoring them might be sufficient.

The set of files changes over time because different version of diff seem to produce them under different circumstances. I have a local CI job which checks that the user did not modify anything created by vendor-libsecp.sh, and I've simply whitelisted anything that ends in .orig.

The CI failure is unrelated to your PR. I'll run my tests on this and ACK/merge it.

@nymius
Copy link
Contributor Author

nymius commented Dec 13, 2024

Thanks for the thorough response. I found this by accident while rebasing a local branch, I executed find -name "*.orig" -delete and this introduced the changes to stage, which surprised me.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 4f36b71; successfully ran local tests

@apoelstra apoelstra merged commit 3b2868d into rust-bitcoin:master Dec 14, 2024
29 of 30 checks passed
@nymius nymius deleted the chore/remove-orig-files-from-version-control branch December 15, 2024 00:48
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