search

indexmap-rs / indexmap

A hash table with consistent order and fast iteration; access items by key or sequence index
https://docs.rs/indexmap/
Other
1.71k stars 150 forks source link

Upgrade to arbitrary 1.3 #271

Closed fornwall closed 1 year ago

fornwall commented 1 year ago

See https://github.com/gfx-rs/naga/pull/2426

daxpedda commented 1 year ago

To summarize: currently arbitrary v1.0.0 doesn't actually compile with the minimum version of syn v1.0.0 they specify themselves. This was (probably unintentionally) fixed in arbitrary v1.3.

This was caught in the -Zminimal-versions check on CI by naga.

cuviper commented 1 year ago

Why wouldn't you just bump to arbitrary 1.3 in naga's dependency?

I also don't understand why this is a new problem for you with indexmap 2...

daxpedda commented 1 year ago

Why wouldn't you just bump to arbitrary 1.3 in naga's dependency?

True, I didn't consider this.

I also don't understand why this is a new problem for you with indexmap 2...

I just dug into this, the way -Zminimal-versions was used in nagas CI before was broken (I will provide a fix soon).

I guess indexmap doesn't need this PR anymore. It would probably be still good for indexmap to actually support building the minimal version it specifies (I'm happy to add a -Zminimal-versions run in the CI as well), if you don't deem it so, feel free to close this.

cuviper commented 1 year ago

I'm on board with making sure that we have correctly specified our direct dependencies, but I'm not fond of checking -Zminimal-versions because of these problems with indirect dependencies. Indexmap does work fine with the API of arbitrary 1.0.0, but that full dependency chain needing syn 1.0.6 is not really our problem.

cuviper commented 1 year ago

Ooh, I hadn't noticed -Z direct-minimal-versions before -- that would be more palatable.

daxpedda commented 1 year ago

I'm on board with making sure that we have correctly specified our direct dependencies, but I'm not fond of checking -Zminimal-versions because of these problems with indirect dependencies. Indexmap does work fine with the API of arbitrary 1.0.0, but that full dependency chain needing syn 1.0.6 is not really our problem.

I'm unsure though what arbitrary should do at this point though, yank all their old versions? arbitary v1.3 actually works correctly. I don't disagree though, it's a problem.

I'm gonna make a new PR then.

cuviper commented 1 year ago

I'm unsure though what arbitrary should do at this point though, yank all their old versions? arbitary v1.3 actually works correctly.

Nah, it doesn't need to be yanked. It's a bug, but not a showstopper, only affecting users that want -Zminimal-versions to work. Such users can update to their requirement to the fixed version 1.3.

And I understand that there's not also an indexmap version that you could update to solve this from afar, but anyone using indexmap/arbitrary should also have their own arbitrary dependency that can require newer.

Closing in favor of #272, thanks!