search

FuelLabs / sway

🌴 Empowering everyone to build reliable and efficient smart contracts.
https://docs.fuel.network/docs/sway/
Apache License 2.0
62.66k stars 5.36k forks source link

Resolve difference between `http` and `https` in forc dependency URL declarations #931

Open mitchmindtree opened 2 years ago

mitchmindtree commented 2 years ago

As spotted in https://github.com/FuelLabs/fuels-rs/pull/150, currently forc will consider the same package declared with both http and https at different branches in the package graph as distinct dependencies. This means that even if they ultimately fetch the same git reference and pin to the same commit, forc will track and build them separately in the dependency graph. This doesn't appear to cause any bugs just yet, though results in extra bloat and may potentially cause issues with traits in the future.

It looks like cargo solves this by resolving both to the same URL, preferring the higher-level package's URL in the case that one uses http and the other https. That is, if A depends on B and both A and B depends on C, A's declaration of C is used. This is the case even if A's dependency on C is the weaker security preference. E.g. if A specifies the dependency on C with a http URL and B specifies the dependency on C with a https URL, B's request for C to be fetched over TLS is ignored.

I propose that we take a similar approach to cargo, but rather than selecting the URL based on the higher-level package, we instead select the stronger security approach. That is, if one package specifies http and another specifies https, we always prefer https.

adlerjohn commented 2 years ago

Instead of http/https, could we not enforce git?

mitchmindtree commented 2 years ago

Good question! I'm not familiar enough with the differences, or sure what the limitations are w.r.t only supporting the git protocol. We should look into this.

It might catch some git users / Rustaceans off guard if we choose to only support a subset of the URLs that git does. Then again, maybe not a big issue if we have a descriptive error describing how to translate the URL to a git one.

As a side note, I think git itself supports quite a few different protocols, and this issue glosses over that fact. E.g. what if the same lib is specified via http, https, git and ssh under different packages? It's unlikely, but maybe hints at there being some better approach to URL normalisation out there... We should take a closer look at cargo's handling of git dep URLs.