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

Add a auto-install feature #6

Open
erik-krogh opened this issue Dec 14, 2020 · 2 comments
Open

Add a auto-install feature #6

erik-krogh opened this issue Dec 14, 2020 · 2 comments
Labels
cli Features that are visible directly through bin/cli enhancement New feature or request tools Features that are implemented in contrib/tools

Comments

@erik-krogh
Copy link
Contributor

My suggestion is to add a bin/cli auto-install eslint command, which would:

  • Run an auto-install.ts script inside the contrib/tools/eslint folder. This script would:
  • Run the install.sh script, with the install folder set to a default root/tools/eslint.
    (Possibly prombt the user about
  • Add the resulting configuration to config.json, possibly creating the config.json file if it doesn't exist.

The auto-install.ts file should satisfy some interface.
E.g:

declare module autoinstall {
  export async function doInstall(installFolder: string): {bin: string, args: string[], options: string[]}
}

Where the return-value from doInstall is the entry that should be added the tools property in config.json.

The bin/cli tools command would also inform which tools can be auto-installed.

@erik-krogh erik-krogh added the enhancement New feature or request label Dec 14, 2020
@esbena
Copy link
Contributor

esbena commented Dec 16, 2020

I like the suggestion. This would make it much easier to get started.


Here are my thoughts on the design (I am not pointing my finger at anyone specific to implement this yet, feel free to volunteer):

My suggestion is to add a bin/cli auto-install eslint command, which would:

The command could support nargs: bin/cli auto-install eslint nodejsscan codeql. Printing a summary at the end about any failures, in addition to the configuration.

  • Run an auto-install.ts script inside the contrib/tools/eslint folder.

How about using contrib/tools/<tool>/src/auto-installers/<tool-id>.ts? Then there could be auto-installers for multiple versions and variants of a tool, without bloating the public bin/cli auto-install interface.

  • Run the install.sh script, with the install folder set to a default root/tools/eslint.

The default could be work/tool-installations - next to all the other auto-generated directories.
That directory could be configurable as --tool-installations or in config.json as { "tool-installations": "/my-dir" }.
I am not quite satisfied with the verbose "tool-installations" name, but "tools" is already taken :).

  • Add the resulting configuration to config.json, possibly creating the config.json file if it doesn't exist.

I would be careful about updating an existing config.json automatically since that may change the formatting if we do it naively, and worse: delete the config.json unless we make a backup or atomic write.
How about:

  • printing the resulting config.json to stdout
  • writing the resulting config.json to work/generated-config.json
  • writing the resulting config.json to config.json if it does not exist already (or if a an explicit flag --update-config flag is added to the command)

We also need to think about how to deal with conflicts: both for the .tools.<tool-id> property names, and for the files in the installation directory. I think bailing early with an error is fine. We can add support for --force later if we find the need for resolving conflicts the other way around.

The auto-install.ts file should satisfy some interface. ... Where the return-value from doInstall is the entry that should be added the tools property in config.json.

Yep. Lets keep it simple like that. We do not want to create an ad hoc package system with lots of configurability.

The .tools.<tool-id> property name should be the <tool-id> of the installation script auto-installers/<tool-id>.ts

So, if we have contrib/tools/eslint/src/auto-installers/eslint-2019.ts, then the resulting config file could contain:

{ 
  "tools": {
    "eslint-2019": ...
  }
}

The bin/cli tools command would also inform which tools can be auto-installed.

Yep.

@esbena esbena added cli Features that are visible directly through bin/cli tools Features that are implemented in contrib/tools labels Dec 16, 2020
@esbena
Copy link
Contributor

esbena commented Dec 17, 2020

I have some additional thoughts on this:

  • I think install is better than auto-install naming wise
  • I would prefer to support arbitrary installers (still implemented as JavaScript)
  • I would prefer to support (optional) custom tool-ids and (optional) custom arguments for more advanced usage
    • for instance, a CI setup may want to install three versions of the same analysis tool, and we may as well support that as long as all of the complexity is hidden in the tools/contrib/src/install.ts script

This can be achieved with the following example bin/cli usages:

# basic usage
# knows by convention that `eslint` means `tools/contrib/eslint/src/install.ts`, which is "requirable" at `build/ts/tools/contrib/eslint/src/install.js`
bin/cli install eslint

# custom tool id
bin/cli install --tool eslint-default eslint

# custom installer - a path with slashes gets interpreted different than a simple name like `eslint`
bin/cli install --tool cool-analysis /home/users/private-drivers/cool-analysis/build/install.js

# custom args
jq -n '.version = "2019"' > args.json
bin/cli install --tool eslint-2019 --args args.json eslint

This requires a minor change to the autoinstall type:

declare module installer {
  export async function doInstall(installFolder: string, args?: any /* from `JSON.parse(args)` */): {bin: string, args: string[], options: string[]}
}

I have dropped the nargs suggestion. To get a config.json with multiple tools, --update-config can simply be used at each installation.

bin/cli install --update-config eslint
bin/cli install --update-config codeql
bin/cli install --update-config nodejsscan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Features that are visible directly through bin/cli enhancement New feature or request tools Features that are implemented in contrib/tools
Projects
None yet
Development

No branches or pull requests

2 participants