Update bitflags dependency to v2

[dextero]

Currently, cargo tree run shows the crate depending on both bitflags v1 (directly) and v2 (through dependencies). However, the crate seems to compile just fine with bitflags v2.

[MarijnS95]

While it may compile fine on the surface, the public bitflags API changed significantly enough to need extra code changes. I recommend you to take a look at the corresponding changelog items for a migration guide.

[dextero]

Thank you for taking a look, and for the fast response!

I added:

• Explicit derive for some builtin traits,
• A definition for from_bits_unchecked that got replaced in bitflags by safe from_bits_retain. I marked that one as deprecated.

I checked the API surface compatibility with gbm v0.14.2 by running cargo-semver-checks. It found no issues with the updated PR.

I also generated a diff that highlights changes to the public API of BufferObjectFlags using cargo-public-api.

The output lists no deletes, a handful of additions and one change: making BufferObjectFlags a tuple struct (in v1, it was a struct with private bits field).

Besides the changes highlighted by the tooling above, I didn't find other updates that could be needed by going through the changelog items (unfortunately I couldn't find a proper migration guide).

I'd be grateful if let could me know if I missed any other checks that you usually do, or if this doesn't seem like a worthwhile effort at all.

[Drakulix]

While I think this should mostly be compatible with downstream, looking at the diff and the bitflags 2.0 release, it seems like the generated struct doesn't have a .bits fields anymore, so this is technically breakage.

I might just merge this, but wait for a future 0.15 release to actually include it.
Which isn't that unlikely anyway, since we really should address #25.

[MarijnS95]

Thank you for taking a look, and for the fast response!

Thanks for picking this up with mininmal info!

I don't think GBM absolutely needs semver-compatibility for these things as it's still in the 0. stage. Just making sure (with my comment above) that "expected" boilerplate APIs like Debug, Eq etc persist on the interface :)

Besides the changes highlighted by the tooling above, I didn't find other updates that could be needed by going through the changelog items (unfortunately I couldn't find a proper migration guide).

It doesn't seem to have been merged there, but it was edited into the GitHub release tag (https://github.com/bitflags/bitflags/releases/tag/2.0.0) after I created an issue about not having an immediate clue about these "hidden" changes.

Specifically, I'd look at the serialization flags. A tad annoying because it might imply an optional dependency on serde though we never know if an external crate is using it that way.

(Perhaps worth making an issue about the Changelog.md being so massively desynced with the much-more-complete GitHub release pages?)

I might just merge this, but wait for a future 0.15 release to actually include it.

If we do, we might as well drop the deprecated backwards-compatible from_bits_retain() helper function straight away.

[dextero]

It doesn't seem to have been merged there, but it was edited into the GitHub release tag (https://github.com/bitflags/bitflags/releases/tag/2.0.0) after I created an issue about not having an immediate clue about these "hidden" changes.

Welp, one more place to look for info in the future, thanks for sharing! It's useful to know a bit better in how many ways one can break compatibility with seemingly minor changes 😅

Specifically, I'd look at the serialization flags. A tad annoying because it might imply an optional dependency on serde though we never know if an external crate is using it that way.

I added optional serde feature that makes BufferObjectFlags derive serde macros as suggested in bitflags release notes.

If we do, we might as well drop the deprecated backwards-compatible from_bits_retain() helper function straight away.

👍 done too.

[dextero]

Annnnd it turns out bitflags with serde feature doesn't compile with serde < 1.0.103, even though it declares the dependency as just 1.0, so a -Z minimal-versions check fails. It's pretty cool this got caught.

I'll suggest a patch in bitflags, but all currently published versions are already broken. Hopefully a workaround from the last commit is fine?

[MarijnS95]

Yeah, -Z minimal-versions is really powerful. I try to have it in all my crates, both to provide a good base layer to (unfortunately few) users that care about this, and to prove that there's always at least some combination of crate dependencies that satisfies an MSRV, even if (in)direct dependencies might bump their MSRV and break ours some time in the future.

A patch in bitflags bumping the minimum serde version is preferable (even if only landing here in the long term), together with a similar CI run to catch such mistakes in the future.

---

I'll leave it to the gbm.rs maintainers to decide if the nice of serializing its bitflags through serde is worthy of having the optional serde dependency in gbm.rs.

[dextero]

A patch in bitflags bumping the minimum serde version is preferable (even if only landing here in the long term), together with a similar CI run to catch such mistakes in the future.

bitflags/bitflags#404

[Drakulix]

I'll leave it to the gbm.rs maintainers to decide if the nice of serializing its bitflags through serde is worthy of having the optional serde dependency in gbm.rs.

If there is anyone, who actually needs this, they will be delighted. And given it is not enabled by default, I don't see any issue with this.

Thanks for all the work, I think this is good for merge now.