Skip to content

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
chain-forgexcr45 added a commit to chain-forgexcr45/rust-secp256k1 that referenced this pull request Sep 28, 2025
…on control

4f36b717c5e88a630d86fb51b3797e1aea73251d style: add rule to ignore *.orig files in .gitignore (nymius)
aebf8bccdec4049e3533f1a818373e12a7e14f8c chore: remove *.orig files from version control (nymius)

Pull request description:

  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`](https://git-scm.com/docs/git-mergetool#_temporary_files), 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](https://github.com/bitcoin-core/secp256k1) 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.

ACKs for top commit:
  apoelstra:
    ACK 4f36b717c5e88a630d86fb51b3797e1aea73251d; successfully ran local tests

Tree-SHA512: de652ec35b7bced4795b90fc1956a2d4bccdcc838c8e2d7f508d6009d0f3765e14c4cc4132f5cb8152e4cf8e7e1686aca703b2fdd6cfb4e381a1d080d4ea1a78
william2332-limf added a commit to william2332-limf/rust-secp256k1 that referenced this pull request Oct 2, 2025
…on control

4f36b717c5e88a630d86fb51b3797e1aea73251d style: add rule to ignore *.orig files in .gitignore (nymius)
aebf8bccdec4049e3533f1a818373e12a7e14f8c chore: remove *.orig files from version control (nymius)

Pull request description:

  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`](https://git-scm.com/docs/git-mergetool#_temporary_files), 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](https://github.com/bitcoin-core/secp256k1) 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.

ACKs for top commit:
  apoelstra:
    ACK 4f36b717c5e88a630d86fb51b3797e1aea73251d; successfully ran local tests

Tree-SHA512: de652ec35b7bced4795b90fc1956a2d4bccdcc838c8e2d7f508d6009d0f3765e14c4cc4132f5cb8152e4cf8e7e1686aca703b2fdd6cfb4e381a1d080d4ea1a78
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