Comments (3)
Ok after some more digging it seems that the
Url
crate is where things are breaking.Local device paths are not supported in the Url crate on Windows (see this issue)
If I rewrite the
with_root
function here to exclude any paths created using theUrl
crate and simply use rusts' path joining as below then everything runs correctly on UNIX systems and Windows.Ok(Source::local(root.as_ref().join(source.as_ref())))
@digorithm what are your thoughts on this? One option would be to remove the option to load an ABI from a URL entirely as it's not guaranteed that the URL exists. The other option which feels a bit hacky is to check if
#[cfg(target_os = "windows")]
and skip trying to create aPathBuf
using Url and instead just use the above method.Keen to hear your thoughts.
First option sounds really good. I'm all for not supporting URLs. Like you both said, lots of potential issues when handling URLs in this case (e.g. in macro expansion).
The original rationale of supporting URLs was just to keep a light feature parity with ethers-rs. But this seems more trouble than what it is worth. So feel free to simplify; supporting more platforms > supporting URLs, IMO.
from fuels-rs.
I think we should be very careful about our intentions to support URLs in the future (especially during proc macro expansion), and should consider avoiding it altogether.
Cargo works hard at ensuring builds are reproducible with Cargo.lock
, but if we support fetching from arbitrary URLs during macro expansion we run the risk of allowing users to get into situations where their builds break easily because they happen to be offline, or because the contract at the URL happened to change slightly, etc
My inclination would be to remove url
and code related to Url
sources for now until we work out a safer / more reproducible-friendly way of supporting it.
Further ramblings about cargo
While cargo's support for arbitrary Rust during build scripts and macro expansion is very powerful, I think allowing for arbitrary I/O is widely considered a bit of a mistake. It makes reliable caching of builds nearly impossible and makes it incredibly difficult to ensure that Rust projects are truly reproducible without constraining the whole build environment using fancy shell tricks like Nix does.
I would like to have seen cargo add support for external inputs (like remote files) by declaring them in the manifest file similarly to how we already do for [dependencies]
so that 1. they could be fetched while the rest of the dependencies are being fetched and 2. they can be pinned alongside the rest of the crate dependencies.
from fuels-rs.
Ok after some more digging it seems that the Url
crate is where things are breaking.
Local device paths are not supported in the Url crate on Windows (see this issue)
If I rewrite the with_root
function here to exclude any paths created using the Url
crate and simply use rusts' path joining as below then everything runs correctly on UNIX systems and Windows.
Ok(Source::local(root.as_ref().join(source.as_ref())))
@digorithm what are your thoughts on this? One option would be to remove the option to load an ABI from a URL entirely as it's not guaranteed that the URL exists. The other option which feels a bit hacky is to check if #[cfg(target_os = "windows")]
and skip trying to create a PathBuf
using Url and instead just use the above method.
Keen to hear your thoughts.
from fuels-rs.
Related Issues (20)
- Default test harness uses outdated version
- Merging `fuels-types` and `fuels-core`
- [RSDK-44] Test issue to test GH integration with Linear
- [RSDK-45] [RSDK-46] Test issue to test integration with linear
- Error when I'm building container with rust sdk HOT 1
- Simulate Call as ContractID from WalletUnlocked? HOT 11
- Support `.trim()` for `SizedAsciiString` HOT 5
- deps: change the `forc` version in CI after a new release supports `fuel-core 0.18` HOT 2
- TransactionResponse uses fuel_tx Transaction type
- Change contract methods to accept Into<Address> and Into<ContractId> instead of Address and ContractId
- Abstract away the need for setting witness indexes
- Support `fuels --no-default-features` not including `fuel-tx`'s default dependencies HOT 7
- Add support for sway's `U128` type
- refactor: have a nice UX with `ConsensusParameters` and `ChainConfig`
- docs: update tests to the latest beta endpoint HOT 1
- refactor: remove `set_provider` method from `ViewOnlyAccount` trait HOT 2
- Start a node from an initial database state
- Update sign_tx_and_verify test
- remove ambiguity when importing `WalletUnlocked`
- Consider exposing `FullProgramABI` either by making it public or taking it out to fuel-abi-types HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from fuels-rs.