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

Auto SC commit deletes and recreates files instead of moving them #2

Open
thialfihar opened this issue Feb 26, 2014 · 4 comments
Open

Comments

@thialfihar
Copy link

Heyo. Cool project!

I have some thoughts on commit 9a2b105 -- you delete org/bouncycastle files and then create them fresh in org/spongycastle. The result is a huge diff and the loss of verification that none of the sources were changed.

Of course I'm not saying you can't be trusted, but given the nature of this library, it's worth considering this.

Am I correct in assuming that you keep rebasing your changes on top of any changes coming from bcgit/bc-java? Because then you could change your auto SC commit to use "git mv", which will do the right thing and track changes across the files and show in the diff that they remain unchanged.

An example, since I recently had to move some files as well:
thialfihar/apg@0bf3d1b
(note the moved files after the handful of changes in the beginning).

@dschuermann
Copy link

I strongly support this issue.
This would definitely increase the trust into this library.

@rtyley
Copy link
Owner

rtyley commented Apr 9, 2014

I have some thoughts on commit 9a2b105 -- you delete org/bouncycastle files and then create them fresh in org/spongycastle. The result is a huge diff and the loss of verification that none of the sources were changed.

Yup, the diff is huge, but... see below.

Of course I'm not saying you can't be trusted, but given the nature of this library, it's worth considering this.

Yeah, please don't worry about my feelings on the trust issue. Depending on the level of paranoia you need to attain, there is obviously no reason you should trust me or any artifacts I create, beyond what you can verify yourself.

Am I correct in assuming that you keep rebasing your changes on top of any changes coming from bcgit/bc-java? Because then you could change your auto SC commit to use "git mv", which will do the right thing and track changes across the files and show in the diff that they remain unchanged.

So, from your comment I can see how you (very reasonably) think git mv works, but as it happens, your assumption is not correct - using git mv would make no difference whatsoever.

Each Git commit you make is an immutable complete snapshot of the entire project file tree at that point in time - and that's pretty much all it is. Git does not add any special metadata to the commit to track that one file has moved from one place to another - whether you use git mv or not.

When you look at a diff in Git, and Git tells you a file has been moved, rather than deleted and then re-created somewhere else, it's because Git's performed rename detection. The bigger the diff and the more substantially changed the files, the less likely it is that Git will detect the file-rename with the effort allowed by the default threshold. In the Spongy Castle rename, a massive number of files are changed, and so you will have to set the thresholds quite aggressively in order to get Git to do all the rename detection for you.

Here are a few references on this if you'd rather not take my word for it:

http://stackoverflow.com/a/2314757/438886
http://stackoverflow.com/a/14833041/438886
https://blogs.atlassian.com/2011/10/confluence_git_rename_merge_oh_my/

@thialfihar
Copy link
Author

Yes, you are right about that. But I think the reason the detection cannot be done, is because you immediately change the files. So what would be nice, is one commit that uses git mv, without any changes, then git can track the renaming... and the next commit could do all the necessary changes like the package name and imports.

I still think that would help immensely in seeing what changed when.

@jbuhacoff
Copy link

jbuhacoff commented Jul 14, 2019

To solve the trust & code review issue, you can build your own BC/SC jars from the bc-java sources. Links: mybc on GitHub and mybc on NPM. You can review the source for the tool, it's much less work than reviewing the diffs to BC with thousands of lines of changes. See also my comment on issue #34. If it helps please star my repo!

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

No branches or pull requests

4 participants