Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support dependencies between optional components/extensions #1111

Open
Diggsey opened this Issue May 11, 2017 · 3 comments

Comments

3 participants
@Diggsey
Copy link
Contributor

Diggsey commented May 11, 2017

Motivation

The RLS can now be installed as an optional extension via rustup. However, for it to function correctly, it requires the rust-src and rust-analysis extensions to also be installed. Ideally the user could just write rustup component add rls and the dependencies would be taken care of automatically.

Changes to manifest format

Here is a (very cut down) view of what a current manifest might look like in JSON:

(Click to expand)

{
  "date": "2017-04-27",
  "manifest-version": "2",
  "pkg": {
    "rust": {
      "version": "1.17.0 (56124baa9 2017-04-24)",
      "target": {
        "aarch64-unknown-linux-gnu": {
          "available": true,
          "hash": "b437bad2396b91649622340f2089da031d0d51a1c661915806f9e538f5666fc0",
          "url": "https://static.rust-lang.org/dist/2017-04-27/rust-1.17.0-aarch64-unknown-linux-gnu.tar.gz",
          "components": [{
            "pkg": "rust-docs",
            "target": "aarch64-unknown-linux-gnu"
          }],
          "extensions": [{
            "pkg": "rust-src",
            "target": "*"
          }]
        },
        "arm-unknown-linux-gnueabi": {
          "available": true,
          "hash": "b3254c556b3f427724af1f3a112e5ad93cffd61561b0690f54199ca465901a8f",
          "url": "https://static.rust-lang.org/dist/2017-04-27/rust-1.17.0-arm-unknown-linux-gnueabi.tar.gz",
          "components": [{
            "pkg": "rust-docs",
            "target": "arm-unknown-linux-gnueabi"
          }],
          "extensions": [{
            "pkg": "rust-src",
            "target": "*"
          }]
        }
      }
    },
    "rust-docs": {
      "version": "1.17.0 (56124baa9 2017-04-24)",
      "target": {
        "aarch64-unknown-linux-gnu": {
          "available": true,
          "hash": "df345f87803c7e940453f56f3824244454238ac701014c10dbf6ed51b7417545",
          "url": "https://static.rust-lang.org/dist/2017-04-27/rust-docs-1.17.0-aarch64-unknown-linux-gnu.tar.gz"
        },
        "arm-unknown-linux-gnueabi": {
          "available": true,
          "hash": "a775d37f10d168d77f4dacad7284c8afc436c746b1a0ee1cab919a02afc59a24",
          "url": "https://static.rust-lang.org/dist/2017-04-27/rust-docs-1.17.0-arm-unknown-linux-gnueabi.tar.gz"
        }
      }
    },
    "rust-src": {
      "version": "1.17.0 (56124baa9 2017-04-24)",
      "target": {
        "*": {
          "available": true,
          "hash": "330ab4ad23e0cb0148756f878f560d91474d6b4830d748fad6ef72ba777fa648",
          "url": "https://static.rust-lang.org/dist/2017-04-27/rust-src-1.17.0.tar.gz"
        }
      }
    }
  }
}

To track dependencies, each extension or optional component will gain an optional "dependencies" key, which contains an array of objects, defining the name and target of the dependency:

(Click to expand)

        "aarch64-unknown-linux-gnu": {
          "available": true,
          "hash": "b437bad2396b91649622340f2089da031d0d51a1c661915806f9e538f5666fc0",
          "url": "https://static.rust-lang.org/dist/2017-04-27/rust-1.17.0-aarch64-unknown-linux-gnu.tar.gz",
          "components": [{
            "pkg": "rust-docs",
            "target": "aarch64-unknown-linux-gnu"
          }],
          "extensions": [{
            "pkg": "rust-src",
            "target": "*"
          }, {
            "pkg": "rls",
            "target": "aarch64-unknown-linux-gnu",
            "dependencies": [{
              "pkg": "rust-src",
              "target": "*"
            }]
          }]
        },

Changes to rust's build system

Rust's build system will need to understand which components depend on each other, and will need to encode those dependencies into the manifests it produces.

Changes to rustup

Rustup will need to read the dependencies from the manifest, and handle the following cases:

Action: A user installs a component with dependencies.
Expected result: The dependencies are automatically installed before installing the component, if not already installed

Action: A user uninstalls a component which is depended on by another
Expected result: User is prompted to uninstall both components, or cancel

Action: A user upgrades a toolchain which contains a component with new dependencies
Expected result: The new dependencies are automatically installed

Action: A user specifies several components to install, which may depend on each other
Expected result: The operation completes successfully, even though some of the components were "implied" by others.

Action: The user install a component with dependencies, or uninstalls a component on which another depends
Expected result: The CLI will indicate why these additional install/uninstall actions are happening.

cc @brson

@brson

This comment has been minimized.

Copy link
Contributor

brson commented May 12, 2017

This looks great @Diggsey. Thank you. I like that you showed all the rustup cases too. Those should be reflected in the test suite.

Action: A user uninstalls a component which is depended on by another
Expected result: User is prompted to uninstall both components, or cancel

I'm not sure how to handle this, considering that rustup is used for scripting. There are very few interactive rustup commands today, and they are fairly predictable (rustup-init is one). Today rustup component uninstall is non-interactive.

It may be more predictable for scripters here to either have rustup just remove the components that depend on the component being uninstalled, or to print an error, suggesting the user re-run with -y.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented May 12, 2017

@Diggsey Diggsey added this to Toolchains & components in Issue Categorisation May 12, 2017

@jonathandturner

This comment has been minimized.

Copy link

jonathandturner commented May 14, 2017

You know, I'd be fine with it saying "WARNING: component rls is still installed which depends on rust-src"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.