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

[rush] Pass original cwd into custom commands #1247

Open
chrisdothtml opened this issue Apr 24, 2019 · 2 comments
Open

[rush] Pass original cwd into custom commands #1247

chrisdothtml opened this issue Apr 24, 2019 · 2 comments
Labels
enhancement The issue is asking for a new feature or design change

Comments

@chrisdothtml
Copy link

chrisdothtml commented Apr 24, 2019

One of the things I really like about rush is that it's required to be installed globally, and whenever a command is run, it walks up the directory tree looking for rush configuration, allowing for a common cli throughout all directories of the monorepo. This breaks down, however, when trying to use custom scripts from an individual project directory.

Problem

I have a global lint script that, when executed from the monorepo root, will lint all projects in the repo, and when executed from an individual project, will only lint that project. The problem isn't the command getting invoked from the subdirectory, that works fine, the problem is that my script has no reference to the cwd it was invoked from (process.cwd() at script runtime is just the monorepo root).

The problem is illustrated by this repro repo.

Proposed Solution

I'm not proposing for the cwd to be changed; I'm instead proposing to invoke the script with an INVOKED_CWD environment variable. It's a backwards-compatible, opt-in change, and being an environment variable, automatically works with any type of shellCommand being used.

I'm not very familiar with this repo, but from what I can tell, it looks like this change would require something like:

Alternate solution

I'm not entirely sure what purpose the initCwd env variable is currently serving, but it could possibly be repurposed to hold this new value in the INIT_CWD variable instead. The change would be slightly simpler:


Willing to send a PR if it'll speed up the implementation of this feature!

@octogonz octogonz added enhancement The issue is asking for a new feature or design change needs design The next step is for someone to propose the details of an approach for solving the problem labels Apr 25, 2019
@octogonz
Copy link
Collaborator

INIT_CWD is specific to the NPM package manager. There are some notes here. It's designed to facilitate strings containing shell commands that are being used in place of a proper toolchain script. I would recommend against imparting any additional meaning to INIT_CWD.

Instead Rush should provide a well-defined set of environment variables that get passed to commands as a contract. I recall issue #1223 was proposing something similar for the event hooks.

These environment variables are already supported when invoking Rush:

export const enum EnvironmentVariableNames {
  /**
   * This variable overrides the temporary folder used by Rush.
   * The default value is "common/temp" under the repository root.
   */
  RUSH_TEMP_FOLDER = 'RUSH_TEMP_FOLDER',

  /**
   * This variable overrides the version of Rush that will be installed by
   * the version selector.  The default value is determined by the "rushVersion"
   * field from rush.json.
   */
  RUSH_PREVIEW_VERSION = 'RUSH_PREVIEW_VERSION',

  /**
   * This variable selects a specific installation variant for Rush to use when installing
   * and linking package dependencies.  For more information, see this article:
   * https://rushjs.io/pages/advanced/installation_variants/
   */
  RUSH_VARIANT = 'RUSH_VARIANT',

  /**
   * If this variable is set to "true", Rush will create symlinks with absolute paths instead
   * of relative paths. This can be necessary when a repository is moved during a build or
   * if parts of a repository are moved into a sandbox.
   */
  RUSH_ABSOLUTE_SYMLINKS = 'RUSH_ABSOLUTE_SYMLINKS'
}

I'm thinking there should be a different prefix for parameters passed when invoking a custom command. Perhaps something like RUSHCMD_INVOKED_CWD.

And then for the hook parameter, maybe it would be RUSHHOOK_COMMAND and RUSHHOOK_PARAMETERS.

That way we have separate namespaces to clearly indicate the scopes where the particular variable apply.

@iclanton @lahuey what do you think?

@chrisdothtml
Copy link
Author

These environment variables are already supported

Ah, cool, didn't know those other variables existed. My initial thought was to use the RUSH_ prefix until I saw INIT_CWD, but I think the scoped prefixes make a lot of sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is asking for a new feature or design change
Projects
Status: Needs triage
Development

No branches or pull requests

2 participants