search

streamingfast / substreams-rs

7 stars 3 forks source link

Linking issue when building with Windows #1

Closed robinbernon closed 1 year ago

robinbernon commented 1 year ago

I get the following issue when building your Subtreams-UniswapV3 repo using Windows 10 with Visual Studio 2019 MSVC:

libsubstreams-1b981aa0f38db442.rlib(substreams-1b981aa0f38db442.substreams.efa1fb51-cgu.12.rcgu.o) : error LNK2019: unresolved external symbol __imp_delete_prefix referenced in function _ZN10substreams5state13delete_prefix17h0ddc1be9ab2a4380E libsubstreams-1b981aa0f38db442.rlib(substreams-1b981aa0f38db442.substreams.efa1fb51-cgu.14.rcgu.o) : error LNK2019: unresolved external symbol __imp_output referenced in function _ZN10substreams10output_raw17h35e087f7f98c6c0aE libsubstreams-1b981aa0f38db442.rlib(substreams-1b981aa0f38db442.substreams.efa1fb51-cgu.14.rcgu.o) : error LNK2019: unresolved external symbol __imp_register_panic referenced in function _ZN10substreams4hook17hb1ea320c90a49ac0E

I've traced this back to the Substreams-rs repo and seen that it's to do with a linking issue caused by the following linked fns:

Was wandering whether you might have seen this before and if you have ideas for fixes/workarounds for this issue?

maoueh commented 1 year ago

At first sight, this seems because you are not compiling to the right target which is wasm32-unknown-unknown, what is the cargo command you are using and what commit on uniswap v3 are you using?

robinbernon commented 1 year ago

Command used with uniswapV3:

cargo build --target="wasm32-unknown-unknown"

maoueh commented 1 year ago

What is the output of cargo build -vvv --target "wasm32-unknown-unknown"?

robinbernon commented 1 year ago

``set CARGO=\?{home}.rustup\toolchains\1.60.0-x86_64-pc-windows-msvc\bin\cargo.exe&& set CARGO_CRATE_NAME=build_script_build&& set CARGO_MANIFEST_DIR={home}\Documents\messari\substreams-uniswap-v3&& set CARGO_PKG_AUTHORS=""&& set CARGO_PKG_DESCRIPTION="Uniswap v3 Substreams"&& set CARGO_PKG_HOMEPAGE=""&& set CARGO_PKG_LICENSE=""&& set CARGO_PKG_LICENSE_FILE=""&& set CARGO_PKG_NAME=substreams-uniswap-v3&& set CARGO_PKG_REPOSITORY=""&& set CARGO_PKG_VERSION=0.1.0&& set CARGO_PKG_VERSION_MAJOR=0&& set CARGO_PKG_VERSION_MINOR=1&& set CARGO_PKG_VERSION_PATCH=0&& set CARGO_PKG_VERSION_PRE=""&& set CARGO_PRIMARY_PACKAGE=1&& set PATH="{home}\Documents\messari\substreams-uniswap-v3\target\debug\deps;{home}.rustup\toolchains\1.60.0-x86_64-pc-windows-msvc\bin;{home}.rustup\toolchains\1.60.0-x86_64-pc-windows-msvc\bin;......"&& rustc --crate-name build_script_build --edition=2021 build.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 -C metadata=955c21a7ee47bd76 -C extra-filename=-955c21a7ee47bd76 --out-dir {home}\Documents\messari\substreams-uniswap-v3\target\debug\build\substreams-uniswap-v3-955c21a7ee47bd76 -C incremental={home}\Documents\messari\substreams-uniswap-v3\target\debug\incremental -L dependency={home}\Documents\messari\substreams-uniswap-v3\target\debug\deps --extern anyhow={home}\Documents\messari\substreams-uniswap-v3\target\debug\deps\libanyhow-0749003461c591e3.rlib --extern prost_build={home}\Documents\messari\substreams-uniswap-v3\target\debug\deps\libprost_build-a80847001dd92098.rlib --extern substreams_ethereum={home}\Documents\messari\substreams-uniswap-v3\target\debug\deps\libsubstreams_ethereum-39b3f3de8557cd09.rlib

maoueh commented 1 year ago

The target does not seems to be set correctly, not sure why but it's wrong and most probably explains the problem.

  1. I don't see anywhere in the output a mention of --target wasm32-unknown-unknown which seems to be the case when set up correctly (at least on my computer I see it)
  2. Second, the output folder will include wasm32-unknown-unknown when the specific target is specified which is not the case in your output, for example: <project>\substreams-uniswap-v3\target\debug\deps\libanyhow vs <project>/substreams-uniswap-v3/target/wasm32-unknown-unknown/debug/deps/libbase64
robinbernon commented 1 year ago

Yes I was just about to say the same thing. Just thinking about where would be best to amend the issue. Would you recommend looking at implementing a patch in cargo for this? Seems odd it's not already passing down target as an arg to every sub call here...

maoueh commented 1 year ago

Not clear indeed, might be a bug but seems far fetched. Maybe it's how you invoke it?

Latest develop branch on uniswap-v3 has a .cargo/config.toml that should set the correct target by default when none is specified, maybe try develop branch with just cargo build -vvv and see how it goes?

robinbernon commented 1 year ago

Tried and it has the same issue. I noticed that there is definitely a lack of target specification all the way down the complication tree, (eg deps of deps of... not be compiled with target args)

robinbernon commented 1 year ago

So I think I've worked out it's to do with the build.rs file in the substreams UniswapV3 repo. In order for it to be run to generate the abi bindings it needs to be run as an executable so you can't run it using a wasm target. This means it gets run using your default target. I guess for some reason with mac and linux there are no linking issues produced when running the build, however there are when running this with windows, therefore I've just gone and removed the linking dependencies for when you are running a build with a non wasm target. PR: https://github.com/streamingfast/substreams-rs/pull/2

maoueh commented 1 year ago

I just tested this morning on my Windows 10 computer and I compiled substreams-uniswap-v3 to wasm32-unknown-unknown correctly.

Steps I took:

Compilation went smoothly without a problem. Installed Rust version was 1.64. Before compilation the cargo build command downloaded and installed wasm32-unknown-unknown target.

So it seems more like an environmental issue for now. I'm a bit torned about the PR as it makes most of the crate useless. We have plans for a testing framework/unit tests for Substreams where your PR would probably a good first step.

maoueh commented 1 year ago

I'm not sure I understand your problem anymore :) Of course it's not compilable to generate the final executable neither in Windows nor Unix. It does not work on my Mac neither when using cargo build --target aarch64-apple-darwin explicitly.

The build.rs is invoked as part of the build automatically we agree, so that means that on the Windows build I performed, I did ran the build.rs code so it works on Windows without modification so I doubt it's the problem.

Run On Command Expected Actual
Mac OS M1 cargo build Works Works
Mac OS M1 cargo build --target wasm32-unknown-unknown Works Works
Mac OS M1 cargo build --target aarch64-apple-darwin Linking Error Linking Error
Mac OS M1 cargo test Unable to Run Unable to Run
Mac OS M1 cargo test --target aarch64-apple-darwin Works Works
Windows 10 x86-64 cargo build Works Works
Windows 10 x86-64 cargo build --target wasm32-unknown-unknown Works Works
Windows 10 x86-64 cargo build --target x86_64-pc-windows-msvc Linking Error Linking Error
Windows 10 x86-64 cargo test Unable to Run Unable to Run
Windows 10 x86-64 cargo test --target x86_64-pc-windows-msvc Works Linking Error

I'm inclined to say that the fact that the tests don't work on Windows is a Rust limitation here. I would imagine it does not trim off enough code unused in tests mode, but tested with cargo test --release --target x86_64-pc-windows-msvc which I hoped would fix them all, while it reduce considerably the list of missing implementation to 3, there was still linking errors.

If something else then that is not working it's unexpected. Now all that being said, we still want to minimally maintain unit tests support for Windows, so I'm inclined to merge the PR.

robinbernon commented 1 year ago

Hmm.. I just did a full factory reset and re-installation and I still get the same issue here. Anyway, I'm glad you'd like to add it regardless due to the testing fix. I'll also say that stuff like the ABI bindings generator in the substreams-ethereum crate could be useful to be used by command line tools,etc so it would be good to push these changes for that also.

maoueh commented 1 year ago

Closing now that #2 has been merged