Revert "don't export core"

[TheBlueMatt]

This reverts commit 1b36c27.

This broke build of rust-bitcoin. I'm gonna play with it to see if I can get CI to pass without this commit.

[dr-orlovsky]

utACK

[apoelstra]

ack e257592

[RCasatta]

#122 (comment)

Why all this rush?

[dr-orlovsky]

@RCasatta not just in bitcoin core, but in each crate using bitcoin_hashes, otherwise macro would not work. This is bad from macro hygiene point of view and I think @TheBlueMatt solution is supreme, since it has no such requirement.

The rush was caused by broken builds of rust-bitcoin right after bitcoin_hashes release, and since there was no PR opened there in parallel with this one, your comment was probably missed.

[RCasatta]

I didn't say this is worse, I just feel bad I put a clear "this is breaking" comment in five lines PR description that was missed and I haven't had time to comment the revert before the merge and the history will blame my commit. I should have commented also the bump version PR...

btw the macro is not hygienic anyway since it requires Hash trait in scope

also, this wasn't the only option to fix downstream build, wouldn't yanking 0.9.5 would have been sufficient as a first step?

[TheBlueMatt]

Certainly not your fault the rest of us missed it. As for hygiene, we should fix that, but at least it’s no worse than it was.

…

On May 4, 2021, at 06:28, Riccardo Casatta ***@***.***> wrote:  I didn't say this is worse, I just feel bad I put a clear "this is breaking" comment in five lines PR description that was missed and I haven't had time to comment the revert before the merge. I should have commented also the bump version PR... btw the macro is not hygienic anyway since it requires Hash trait in scope — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[sgeisler]

also, this wasn't the only option to fix downstream build, wouldn't yanking 0.9.5 would have been sufficient as a first step?

I have to agree with this very much. Breaking downstream sucks, but the current situation is not much better tbh, there is still a broken version out there and there is rust-bitcoin/rust-bitcoin#602 because of that apparently. Yanking would have been much easier.

[apoelstra]

@RCasatta the break was not that anybody downstream was actually using the exported core. That is the breakage that you warned about and we explicitly decided to live with it.

The break was that we were exporting the new core in our macros, which is part of the general "rust macros cause downstream breakage that unit tests and compile-tests cannot fix" problem. This was not mentioned anywhere in the original PR, by anybody, and that is what we are fixing now.

[apoelstra]

@sgeisler the fuzztest is not downstream breakage. Anything downstream that depends on the fuzztest feature is dangerously and security-compromisingly broken. Yes, we have to update the rust-bitcoin fuzztests because of this. This was expected and uninteresting.

The hygeine issue is part of the general "rust macros cause downstream breakage that unit tests and compile-tests cannot fix" problem.

[RCasatta]

@RCasatta the break was not that anybody downstream was actually using the exported core. That is the breakage that you warned about and we explicitly decided to live with it.

No, I wrote rust-bitcoin would need extern crate core not that someone is using the core re-exported.