We'll want to add a wasm32 build to CI - even if we aren't going to run tests we'll want to know the build isn't broken.

This looks good to me - anything you're still planning to add or should we merge?

I still want to see if the addition ofresolver = 2 to the workspace is going to cause any issues, it's required for the wasm target because of the[target.'cfg(not(target_arch = "wasm32"))'.dependencies] in thetokio-postgres crate. The property is used from the top level crate that you're compiling but I'm not 100% sure that this is a non-breaking change for people still using the old resolver.

The resolver selection is local to each workspace. Rather than unconditionally enabling thejs feature on getrandom for wasm32, I think it would be better to add ajs feature to postgres-protocol and tokio-postgres. That way we wouldn't be unconditionally locking all wasm users into an assumption of a JS environment if they were instead running in e.g. WASI.

That should remove the need for resolver 2 as well.

The resolver selection is local to each workspace. Rather than unconditionally enabling thejs feature on getrandom for wasm32, I think it would be better to add ajs feature to postgres-protocol and tokio-postgres. That way we wouldn't be unconditionally locking all wasm users into an assumption of a JS environment if they were instead running in e.g. WASI.

That should remove the need for resolver 2 as well.

Agreed about thejs feature but I don't think that's going to be possible with the old feature resolver unless we adddep:socket2 as a default feature because[target.'cfg(not(target_arch = "wasm32"))'.dependencies] isn't respected in the old feature resolver causingsocket2 to get included for all targets, including wasm. I have a branchhere with those changes if we want to proceed down this path.

The current solution of excluding socket2 on non-wasm targets still works as expected for projects using resolver 1 because resolver 1 includes all the dependencies regardless of target. If adding an extra default feature is off the table this could be a workaround where resolver 2 is required for wasm support but not necessary on other platforms.

The socket2 dependency can be unconditionally disabled on wasm - the only thing gated by the feature would be the getrandom feature (which is a no-op on not-wasm so it shouldn't matter if people are using resolver 2 or not).

Yeah I agree with you on puttinggetrandom behind a feature, but which approach do you prefer for making socket2 an optional dependency depending on the target.

The two approaches outlined would be making it a default feature, which would be a breaking change for people using tokio-postgres without default features. Or adding resolver 2 back to the workspace and requiring people using WASM to use the new resolver in their workspace as well, introducing no breaking change for non-wasm users.

[target.'cfg(not(target_arch="wasm32"))'.dependencies]socket2 = {version ="0.5",features = ["all"] }

should be sufficient without any resolver customization I think?

[target.'cfg(not(target_arch="wasm32"))'.dependencies]socket2 = {version ="0.5",features = ["all"] }

should be sufficient without any resolver customization I think?

On resolver 1 that'll still include socket2 for wasm targets becausecfg(not( doesn't seem to be respected on resolver 1. We could keep that in our Cargo.toml but the downstream project would need resolver 2 and we would need resolver 2 in this project (although that's only so we can run CI on wasm and won't affect anyone else). If that's an acceptable tradeoff then I can move this PR out of draft.

I don't think that's correct - resolver 1 vs 2 affects howfeatures of enabled dependencies are unified across cfg-specific dependencies. Platform dependentcrates have always been supported. See for example native-tls which has cfg'd dependencies on schannel and security framework crates that wouldn't build off of their native platform.

Oh wow, yeah I totally had some wires cross in my head 😅. With resolver 1 the issue is that tokio gets compiled with features (including socket2, which is where I made the mistake) that aren't compatible with WASM. By runningcargo check --target wasm32-unknown-unknown --no-default-features --features js -v we eventually end up with cargo running rustc with the tokio like this:

Running `rustc --crate-name build_script_build --edition=2021 /Users/zeb/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.28.2/build.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 --cfg 'feature="bytes"' --cfg 'feature="default"' --cfg 'feature="io-util"' --cfg 'feature="libc"' --cfg 'feature="macros"' --cfg 'feature="mio"' --cfg 'feature="net"' --cfg 'feature="num_cpus"' --cfg 'feature="rt"' --cfg 'feature="rt-multi-thread"' --cfg 'feature="socket2"' --cfg 'feature="sync"' --cfg 'feature="time"' --cfg 'feature="tokio-macros"' --cfg 'feature="windows-sys"' -C metadata=712f08d5e5bd4c22 -C extra-filename=-712f08d5e5bd4c22 --out-dir /Users/zeb/dev/rust/rust-postgres/target/debug/build/tokio-712f08d5e5bd4c22 -L dependency=/Users/zeb/dev/rust/rust-postgres/target/debug/deps --extern autocfg=/Users/zeb/dev/rust/rust-postgres/target/debug/deps/libautocfg-6f9803c3213f701a.rlib --cap-lints allow`

The features are coming from the dev dependencies which isan issue with resolver 1, so to work around that without using resolver 2 just for the CI check I hackily added a sed to ignore the dev dependencies for the workflow.

build's red