FuelLabs / sway

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

Consider using a distinct `forc-workspace.toml` manifest for workspaces #3437

Open mitchmindtree opened 1 year ago

mitchmindtree commented 1 year ago

As raised by @Braqzen on Slack, the role of Forc.toml can appear quite ambiguous to new users, particularly users not coming from the Rust ecosystem.

As of the introduction of workspaces, Forc.toml appears to attempt to fill two roles:

  1. A manifest for a package
  2. A manifest for a workspace (i.e. a set of local packages)

This ambiguity is also reflected in our implementation, e.g. our ManifestFile::from_* constructors first attempt to load a PackageManifestFile. Only upon failure, does it attempt to load a WorkspaceManifestFile. This is reliable for the most part as a workspace will never contain a [project] table, which is required for all PackageManifests, however this does feel a little hacky.

Cargo behaviour

Cargo also makes no distinction between Cargo.toml manifests that are used for workspaces, and those that are used for packages.

Cargo however goes a step further than we do, and actually allows for declaring both a workspace and and package within the same Cargo.toml manifest file.

Like @Braqzen, I'm unconvinced of the usefulness of the mixing of these two concerns. For example, I believe Cargo does not allow for recursively defined workspaces, however the fact that Cargo.toml can define both almost implies that such an approach would be OK. Instead, Cargo tends to produce awkward errors when incorrectly mixing the two (e.g. declaring another workspace in the member or an existing workspace).

sdankel commented 3 weeks ago

I would prefer to follow cargo's approach by keeping the configuration of the workspace inside of Forc.toml.