Upgrade bitvec to 0.21 to unbreak the build

[gnunicorn]

alternative to #250 .
Closes #249 .

bitvec (the reason we have funty in the dependency tree in the first place) has meanwhile released an upgraded version that solves the fix without pinning.

[dvdplm]

@gnunicorn in parity-common we did this and on scale-info I suggested this to fix CI pending an actual upstream fix.

[gnunicorn]

@dvdplm the problem is that this only fixes the local CI but not actually anyone adding parity-scale-codec into their dependencies. (the problem came up trying to test and release the contracts-pallet).

@bkchr I agree with the internal - the alternative is to depend on funty 1.2.0 to get that trait in directly.

. The solution is still to fix this upstream.

I am afraid we won't see any other "upstream fix" to this, considering that the problem arose from a trait defined in bitvec having the BITS (we depend on here), while then adding the same named BITS a trait that depended on causing the issue - and both crates being developed and maintained by the same person, who then released a newer version of the dependent crate removing the field and instead referring to the dependency one.

I fear this is the upstream fix. What other fix are you waiting for?

[athei]

I am not optimistic that there will be a better "upstream fix" soon.

[bkchr]

Yeah with the change to 0.21.0 it looks "okay".

However this is a breaking change, as we have this in our public interface.

I think in the future we need to get rid off this dependency.

[gnunicorn]

by having in our "public interfaces" you mean we are implementing it for that specific version of bitvec? because I can't find us returning any bitvec in our API.

If so, arguably we could have two independent features that implement it for 0.20 and 0.21 respectively (where the earlier would have to pin to 1.1 of funty) - but I guess that's also not gonna help us here, as we activate it through the std-feaure :( ...

[bkchr]

by having in our "public interfaces" you mean we are implementing it for that specific version of bitvec?

This means, that everyone that updates scale codec and also uses bitvec, will need to update their bitvec version. As we are the only ones using it, doing a 2.1.0 release should probably be fine.

[gnunicorn]

As we are the only ones using it, doing a 2.1.0 release should probably be fine.

Sounds reasonable. I am happy to add a changelog entry informing people that the dependency has been upgraded and thus they also need to upgrade the bitvec dependency to get the support...

[athei]

If we do a major version bump we could also remove the implicit addition of bitvec when std is enabled?

[bkchr]

If we do a major version bump we could also remove the implicit addition of bitvec when std is enabled?

If cargo would just support this...

[thiolliere]

PR is good to me

I'm ok to release 2.1, can you write this in the changelog.
I hope ppl will be fine with the breaking change.

[gnunicorn]

@thiolliere already added a changelog entry in the last commit.

[thiolliere]

Sorry to change my mind but actually I don't think we should publish 0.21 like this, some ppl can be affected by this breaking change and I think it can show bad quality image of our libraries.

I open a fix in bitvec which seems good to me ferrilab/bitvec#109

changed opinion, proper fix is to fix bitvec and it seems doable

[ConnorBP]

ahh i just ran into this frustrating build error, good to know it wasn't caused by me on my project.

[athei]

Sorry to change my mind but actually I don't think we should publish 0.21 like this, some ppl can be affected by this breaking change and I think it can show bad quality image of our libraries.

I open a fix in bitvec which seems good to me bitvecto-rs/bitvec#109

This would then be released as a bitvec fix release? Otherwise we would be in the same situation, right?

[thiolliere]

Yes I expect him to release 0.20.2 with it.
If he doesn't I think we can break semver and publish 2.1

[thiolliere]

there is no answer from his side since 4 days,

note that if we release parity-scale-codec with bitvec 0.21, every user who did funty = 1.1.0 will see bitvec not compiling (because bitvec-0.21.0 depends on funty = "1" while it must be funty = 1.2 in order to compile).

I don't know how we can push to have this 0.20.2 release.

I also think that if we use bitvec 0.21 now then nothing guarantee that he doesn't publish a new version of funty and never fix bitvec 0.21.

[athei]

What about dropping the support for bitvec? We are not using it ourselves and this library seems to be trouble.

[thiolliere]

What about dropping the support for bitvec? We are not using it ourselves and this library seems to be trouble.

You mean dropping bitvec in parity-scale-codec 2.1, yes we can do that, and we can provide a few helper function to implement Encode/Decode for bitvec.

I think we use it in polkadot though for some parachain related code.

[athei]

Yes I am talking about removing the dependency with parity-scale-codev 2.1. It seems to be a liability. Not sure how we handle the polkadot case then.

[gnunicorn]

I think we use it in polkadot though for some parachain related code.

Polkadot uses it, and needs these to be codec::Encode/Decode. But we could def. drop bit-vec from being activated through the std-feature and thus by default.

parity-scale-codec/Cargo.toml

Lines 34 to 36 in 1beac25

	default = ["std"]
	derive = ["parity-scale-codec-derive"]
	std = ["serde", "bitvec/std", "byte-slice-cast/std", "chain-error"]

(would also reduce the dependency tree).

[athei]

Isn't that the original problem? That cargo automatically adds a dependency when you activate a feature on it. What we want to express but cannot: std activates bitvec/std but only if bitvec is already activated.

[bkchr]

We can still remove bitvec from the std feature for now. Downstream than probably needs to enable std if required, but there isn't any better solution until weak features are stabilized.

[bkchr]

As said above, I'm also for dropping bitvec in Polkadot. But that requires someone to write some small struct. Shouldn't be that hard.

[thiolliere]

bitvec 0.20.2 is released fixing its dependency. (it now depends on funty = "~1.1") So the issue is solved