Prevent ahash from pulling in getrandom 0.3

[kim]

Modules generally don't honor the workspace cargo lock file, so may upgrade ahash silently, which pulls in a getrandom version that is not the one pulled in via binding. Thus feature unification doesn't apply and modules fail to compile due to the wasm target not being supported by the transitive getrandom.

This also causes smoketests to fail.

Expected complexity level and risk

1

Testing

Smoketests / module compilation fail before this change, and succeed after.

[kim]

Conceivably, this can happen again if we have some other crate that transitively depends on getrandom. Maybe there is a more robust way to enforce a single version of getrandom?

[coolreader18]

I think ideally in the future this kinda thing never happens again in the rust ecosystem, cause it's been a royal pain to deal with. But, also, those errors from getrandom 0.3 are actually exactly what we want - failing to compile when you pull in getrandom. So, this is the right fix.

[coolreader18]

Also, it seems like you unintentionally ran a full cargo update.

[kim]

you unintentionally ran a full cargo update.

It was not quite unintentionally, because the actual problem here is that building a module won't honor the lock file. So making the dependency graphs match, if only for a short period of time, seems like something we should do more often.

Feel free to disagree, but I'd like to understand the reasoning.

[gefjon]

It was not quite unintentionally, because the actual problem here is that building a module won't honor the lock file

Can we (in a separate PR) add a flag to spacetime build and friends which makes it reflect the lockfile? Or possibly just a way to pass cargo flags through more generally?

[kim]

Can we (in a separate PR) add a flag to spacetime build and friends which makes it reflect the lockfile?

No I don't think: a module has spacetimedb in its dependencies, which is what pulled in the faulty dependency graph. I'm not aware of a way to force a dependency to lock its dependencies transitively.

What we might want to consider is to somehow make the tests that compile modules/module-test not honor the lockfile, so we'll catch this earlier and in a more obvious way. I'm not sure this can actually be done except for removing the lockfile.

[gefjon]

Wait, what? Surely if you compile your module with --locked, that makes it respect the lockfile for the entire tree of dependencies including transitive deps, right?

[kim]

If I create a fresh module, it won't have a Cargo.lock file and will go out to the internet to resolve the dependencies. It won't, for a particular dependency, get the lock file of that dependency and consider it for version constraints.

Smoketests create fresh modules all the time, and they are built outside the workspace, so that's why they are failing.

[gefjon]

Ah, I see. Yes, that's unfortunate.

[Centril]

Looks like @coolreader18 and @gefjon got the review handled so no further input from me :)

[kim]

Someone press the button then? 😇