search

rust-bitcoin / rust-bitcoinconsensus

Bitcoin's libbitcoinconsenus.a with Rust binding. Built from Bitcoin sources with cargo.
Apache License 2.0
46 stars 34 forks source link

Move to using git subtree #49

Closed tcharding closed 2 years ago

tcharding commented 2 years ago

Same as #51 but does not use --squash. One should be closed in favour of the other.

Add a git subtree, I could not get the git subtree add command to use a commit hash so I used a tag - this led me to using v0.19.2 instead of the commit hash we are currently on (six commits after tag v0.19.0).

Includes docs in the README and a version bump so this can be merged and release directly if so desired.

Question

Do we want to release a 0.19.2 version or should we just jump straight to 20.2?

Bah, 10,000 commits - I did not notice that. I think we need to sqaush.

apoelstra commented 2 years ago

This looks great. A couple notes:

  1. I'm ambivalent about using --squash ... but I guess here it is the right thing to do. In my experience with subtrees, the subtree repo is much smaller than the main repo (e.g. libsecp inside Bitcoin Core) but here we have the opposite.
  2. I think your trouble with untagged commits came because you directly used github as a remote. I think if you add github.com/bitcoin/bitcoin as an explicit remote (say bitcoin-upstream) then did a git fetch on that, and then did git subtree add --prefix=depend/bitcoin febf04841fd94429ad13c9c1548bd052cdf82833, it'd work. It worked for me locally at least.
  3. Can you use --prefix=depend/bitcoin so that Bitcoin will remain in the same directory? I think you'll also need to add this option to the git submodule pull command.
tcharding commented 2 years ago

I'll think about the other points when next week (saturday hacking session right now)

Can you use --prefix=depend/bitcoin so that Bitcoin will remain in the same directory? I think you'll also need to add this option to the git submodule pull command.

I tried this at first but then we cannot keep every commit building (because one commit has to remove the current depend/bitcoin then the next commit is the one made by git add subtree? If this is important I can have another play with it.

apoelstra commented 2 years ago

For me no existing commits compile, so I wouldn't be heartbroken if one more didn't :).

One possible workaround woulrd be to first rename the submodule, then add the subtree and cut over to it, then delete the submodule, all in different commits.

tcharding commented 2 years ago

Once I clobbered the bitcoin/depend directory it took me a while to work out how to get it back, this works git submodule update --init --recursive (note the --init).

tcharding commented 2 years ago

Leaving this for another day.

tcharding commented 2 years ago

I did as you suggested and used a midway tmp/bitcoin submodule for one commit. I removed the use of --squash as well.

apoelstra commented 2 years ago

Thanks! Will review this later today or tomorrow.

Curious if anybody else here has an opposition to our use of non-squashed subtrees? This does increase the repo size by several dozen times :)

tcharding commented 2 years ago

Curious if anybody else here has an opposition to our use of non-squashed subtrees? This does increase the repo size by several dozen times :)

Don't hold your breath for an answer, not many people watch this crate :)

There is another problem with not squashing. Now running git log --patch @~~.. hangs while git chokes on the size of the patch. That is going to be surprising if people are trying to read patches from arbitrary places before this. I can't remember exactly what it was like with --squash but I can try it again and see.

tcharding commented 2 years ago

I don't know about the size differences, from within rust-bitcoinconsensus running du ., du depend, and du .git all return the same number of Megabytes for this and #51 (with --squash).

The git log --patch issue does not exist for #51 though.

apoelstra commented 2 years ago

The size of depend won't change between the two versions, of course, and your .git directory also won't change in size once you've fetched all the bitcoin commits, unless you entirely re-clone the repo (or somehow convince git to drop the old commits).

I don't understand your comment about git log --patch. What does @~~.. mean?

tcharding commented 2 years ago

I don't understand your comment about git log --patch. What does @~~.. mean?

@ is short for HEAD so @~~ is the last two patches back from HEAD and .. to make it a revision-range.

tcharding commented 2 years ago

Closing in favour of #51