refactor: move environment variable related strings into one class (#648

)
invertase · Feb 25, 2024 · 2db32ec · 2db32ec
1 parent 30611f4
commit 2db32ec
Show file tree

Hide file tree

Showing 9 changed files with 108 additions and 63 deletions.
diff --git a/packages/melos/lib/src/commands/exec.dart b/packages/melos/lib/src/commands/exec.dart
@@ -34,13 +34,14 @@ mixin _ExecMixin on _Melos {
 
     final environment = {
       ...currentPlatform.environment,
-      'MELOS_PACKAGE_NAME': package.name,
-      'MELOS_PACKAGE_VERSION': package.version.toString(),
-      'MELOS_PACKAGE_PATH': package.path,
-      'MELOS_ROOT_PATH': workspace.path,
-      if (workspace.sdkPath != null) envKeyMelosSdkPath: workspace.sdkPath!,
+      EnvironmentVariableKey.melosPackageName: package.name,
+      EnvironmentVariableKey.melosPackageVersion: package.version.toString(),
+      EnvironmentVariableKey.melosPackagePath: package.path,
+      EnvironmentVariableKey.melosRootPath: workspace.path,
+      if (workspace.sdkPath != null)
+        EnvironmentVariableKey.melosSdkPath: workspace.sdkPath!,
       if (workspace.childProcessPath != null)
-        'PATH': workspace.childProcessPath!,
+        EnvironmentVariableKey.path: workspace.childProcessPath!,
     };
 
     if (package.isExample) {
@@ -53,27 +54,16 @@ mixin _ExecMixin on _Melos {
           await readTextFileAsync(exampleParentPubspecPath),
         );
 
-        environment['MELOS_PARENT_PACKAGE_NAME'] = exampleParentPackage.name!;
-        environment['MELOS_PARENT_PACKAGE_VERSION'] =
+        environment[EnvironmentVariableKey.melosParentPackageName] =
+            exampleParentPackage.name!;
+        environment[EnvironmentVariableKey.melosParentPackageVersion] =
             (exampleParentPackage.version ?? Version.none).toString();
-        environment['MELOS_PARENT_PACKAGE_PATH'] = exampleParentPackagePath;
+        environment[EnvironmentVariableKey.melosParentPackagePath] =
+            exampleParentPackagePath;
       }
     }
-    if (environment.containsKey('MELOS_TEST')) {
-      // TODO(rrousselGit) refactor this to not have to manually maitain the
-      // list of env variables to remove
-      environment.remove('MELOS_TEST');
-      environment.remove('MELOS_ROOT_PATH');
-      environment.remove('MELOS_SCRIPT');
-      environment.remove('MELOS_PACKAGE_NAME');
-      environment.remove('MELOS_PACKAGE_VERSION');
-      environment.remove('MELOS_PACKAGE_PATH');
-      environment.remove('MELOS_PARENT_PACKAGE_NAME');
-      environment.remove('MELOS_PARENT_PACKAGE_VERSION');
-      environment.remove('MELOS_PARENT_PACKAGE_PATH');
-      environment.remove(envKeyMelosPackages);
-      environment.remove(envKeyMelosSdkPath);
-      environment.remove(envKeyMelosTerminalWidth);
+    if (environment.containsKey(EnvironmentVariableKey.melosTest)) {
+      EnvironmentVariableKey.allMelosKeys().forEach(environment.remove);
     }
 
     return startCommand(

diff --git a/packages/melos/lib/src/commands/run.dart b/packages/melos/lib/src/commands/run.dart
@@ -90,10 +90,11 @@ mixin _RunMixin on _Melos {
       ..validate();
 
     final environment = {
-      'MELOS_ROOT_PATH': config.path,
-      if (workspace.sdkPath != null) envKeyMelosSdkPath: workspace.sdkPath!,
+      EnvironmentVariableKey.melosRootPath: config.path,
+      if (workspace.sdkPath != null)
+        EnvironmentVariableKey.melosSdkPath: workspace.sdkPath!,
       if (workspace.childProcessPath != null)
-        'PATH': workspace.childProcessPath!,
+        EnvironmentVariableKey.path: workspace.childProcessPath!,
       ...script.env,
     };
 
@@ -148,7 +149,7 @@ mixin _RunMixin on _Melos {
       // MELOS_PACKAGES environment is detected by melos itself when through
       // a defined script, this comma delimited list of package names used to
       // scope the `packageFilters` if it is present.
-      environment[envKeyMelosPackages] = packagesEnv;
+      environment[EnvironmentVariableKey.melosPackages] = packagesEnv;
     }
 
     return startCommand(

diff --git a/packages/melos/lib/src/commands/runner.dart b/packages/melos/lib/src/commands/runner.dart
@@ -18,6 +18,7 @@ import 'package:yaml_edit/yaml_edit.dart';
 
 import '../command_runner/version.dart';
 import '../common/aggregate_changelog.dart';
+import '../common/environment_variable_key.dart';
 import '../common/exception.dart';
 import '../common/git.dart';
 import '../common/git_commit.dart';
@@ -82,13 +83,14 @@ abstract class _Melos {
   }) async {
     var filterWithEnv = packageFilters;
 
-    if (currentPlatform.environment.containsKey(envKeyMelosPackages)) {
+    if (currentPlatform.environment
+        .containsKey(EnvironmentVariableKey.melosPackages)) {
       // MELOS_PACKAGES environment variable is a comma delimited list of
       // package names - used to scope the `packageFilters` if it is present.
       // This can be user defined or can come from package selection in
       // `melos run`.
       final filteredPackagesScopeFromEnv =
-          currentPlatform.environment[envKeyMelosPackages]!
+          currentPlatform.environment[EnvironmentVariableKey.melosPackages]!
               .split(',')
               .map(
                 (e) => createGlob(e, currentDirectoryPath: config.path),

diff --git a/packages/melos/lib/src/common/environment_variable_key.dart b/packages/melos/lib/src/common/environment_variable_key.dart
@@ -0,0 +1,44 @@
+class EnvironmentVariableKey {
+  EnvironmentVariableKey._();
+
+  static const String melosRootPath = 'MELOS_ROOT_PATH';
+  static const String melosPackageName = 'MELOS_PACKAGE_NAME';
+  static const String melosPackageVersion = 'MELOS_PACKAGE_VERSION';
+  static const String melosPackagePath = 'MELOS_PACKAGE_PATH';
+  static const String melosParentPackageName = 'MELOS_PARENT_PACKAGE_NAME';
+  static const String melosParentPackageVersion =
+      'MELOS_PARENT_PACKAGE_VERSION';
+  static const String melosParentPackagePath = 'MELOS_PARENT_PACKAGE_PATH';
+  static const String melosScript = 'MELOS_SCRIPT';
+  static const String melosTest = 'MELOS_TEST';
+
+  /// This user-defined environment variable contains a comma delimited list of
+  /// package names that Melos should focus on. This will act as the global
+  /// `scope` package filter, and it will override the `scope` for all the
+  /// filtering options defined in the `packageFilters` section.
+  static const String melosPackages = 'MELOS_PACKAGES';
+
+  /// This user-defined environment has a path to the Dart/Flutter SDK to use.
+  /// This environment variable has precedence over the `sdkPath` option in
+  /// `melos.yaml`, but is overridden by the command line option `--sdk-path`.
+  static const String melosSdkPath = 'MELOS_SDK_PATH';
+
+  static const String melosTerminalWidth = 'MELOS_TERMINAL_WIDTH';
+
+  static const String path = 'PATH';
+
+  static List<String> allMelosKeys() => [
+        melosRootPath,
+        melosPackageName,
+        melosPackageVersion,
+        melosPackagePath,
+        melosParentPackageName,
+        melosParentPackageVersion,
+        melosParentPackagePath,
+        melosScript,
+        melosTest,
+        melosPackages,
+        melosSdkPath,
+        melosTerminalWidth,
+      ];
+}
diff --git a/packages/melos/lib/src/common/utils.dart b/packages/melos/lib/src/common/utils.dart
@@ -32,6 +32,7 @@ import 'package:yaml/yaml.dart';
 import '../logging.dart';
 import '../package.dart';
 import '../workspace.dart';
+import 'environment_variable_key.dart';
 import 'exception.dart';
 import 'io.dart';
 import 'platform.dart';
@@ -76,15 +77,6 @@ extension Let<T> on T? {
 /// ```
 String multiLine(List<String> lines) => lines.join('\n');
 
-// MELOS_PACKAGES environment variable is a comma delimited list of
-// package names - used to scope the `packageFilters` if it is present.
-// This can be user defined or can come from package selection in `melos run`.
-const envKeyMelosPackages = 'MELOS_PACKAGES';
-
-const envKeyMelosSdkPath = 'MELOS_SDK_PATH';
-
-const envKeyMelosTerminalWidth = 'MELOS_TERMINAL_WIDTH';
-
 final melosPackageUri = Uri.parse('package:melos/melos.dart');
 
 final _camelCasedDelimiterRegExp = RegExp(r'[_\s-]+');
@@ -140,9 +132,11 @@ extension StringUtils on String {
 }
 
 int get terminalWidth {
-  if (currentPlatform.environment.containsKey(envKeyMelosTerminalWidth)) {
+  if (currentPlatform.environment
+      .containsKey(EnvironmentVariableKey.melosTerminalWidth)) {
     return int.tryParse(
-          currentPlatform.environment[envKeyMelosTerminalWidth]!,
+          currentPlatform
+              .environment[EnvironmentVariableKey.melosTerminalWidth]!,
           radix: 10,
         ) ??
         80;
@@ -444,13 +438,13 @@ Future<Process> startCommandRaw(
   return Process.start(
     executable,
     currentPlatform.isWindows
-        ? ['/C', '%MELOS_SCRIPT%']
-        : ['-c', r'eval "$MELOS_SCRIPT"'],
+        ? ['/C', '%${EnvironmentVariableKey.melosScript}%']
+        : ['-c', 'eval "\$${EnvironmentVariableKey.melosScript}"'],
     workingDirectory: workingDirectory,
     environment: {
       ...environment,
-      envKeyMelosTerminalWidth: terminalWidth.toString(),
-      'MELOS_SCRIPT': command.join(' '),
+      EnvironmentVariableKey.melosTerminalWidth: terminalWidth.toString(),
+      EnvironmentVariableKey.melosScript: command.join(' '),
     },
     includeParentEnvironment: includeParentEnvironment,
   );
@@ -638,7 +632,8 @@ List<List<Package>> findCyclicDependenciesInWorkspace(List<Package> packages) {
 /// / dart pub / flutter pub.
 ///
 /// Takes into account a potential sdk path being provided. If no sdk path is
-/// provided then it will assume to use the pub command available in PATH.
+/// provided then it will assume to use the pub command available in
+/// [EnvironmentVariableKey.path].
 List<String> pubCommandExecArgs({
   required bool useFlutter,
   required MelosWorkspace workspace,

diff --git a/packages/melos/lib/src/package.dart b/packages/melos/lib/src/package.dart
@@ -27,6 +27,7 @@ import 'package:pool/pool.dart';
 import 'package:pub_semver/pub_semver.dart';
 import 'package:pubspec/pubspec.dart';
 
+import 'common/environment_variable_key.dart';
 import 'common/exception.dart';
 import 'common/git.dart';
 import 'common/glob.dart';
@@ -656,8 +657,10 @@ extension on Iterable<Package> {
         // variables
         // TODO(rrousselGit): should support environment variables other than
         // PACKAGE_NAME
-        final expandedFileExistsPath =
-            fileExistsPath.replaceAll(r'$MELOS_PACKAGE_NAME', package.name);
+        final expandedFileExistsPath = fileExistsPath.replaceAll(
+          '\$${EnvironmentVariableKey.melosPackageName}',
+          package.name,
+        );
 
         return fileExists(p.join(package.path, expandedFileExistsPath));
       });

diff --git a/packages/melos/lib/src/workspace.dart b/packages/melos/lib/src/workspace.dart
@@ -20,6 +20,7 @@ import 'dart:async';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
 
+import 'common/environment_variable_key.dart';
 import 'common/intellij_project.dart';
 import 'common/io.dart';
 import 'common/platform.dart';
@@ -85,7 +86,8 @@ class MelosWorkspace {
       dependencyOverridePackages: dependencyOverridePackages,
       sdkPath: resolveSdkPath(
         configSdkPath: workspaceConfig.sdkPath,
-        envSdkPath: currentPlatform.environment[utils.envKeyMelosSdkPath],
+        envSdkPath:
+            currentPlatform.environment[EnvironmentVariableKey.melosSdkPath],
         commandSdkPath: global?.sdkPath,
         workspacePath: workspaceConfig.path,
       ),
@@ -130,8 +132,9 @@ class MelosWorkspace {
 
   /// Returns the path to a [tool] from the Dart/Flutter SDK.
   ///
-  /// If no [sdkPath] is specified, this will return the name of the tool as is
-  /// so that it can be used as an executable from PATH.
+  /// If no [sdkPath] is specified, this will return the name of the tool
+  /// as is so that it can be used as an executable from
+  /// [EnvironmentVariableKey.path].
   String sdkTool(String tool) {
     final sdkPath = this.sdkPath;
     if (sdkPath != null) {
@@ -140,15 +143,17 @@ class MelosWorkspace {
     return tool;
   }
 
-  /// PATH environment variable for child processes launched in this workspace.
+  /// [EnvironmentVariableKey.path] environment variable for child processes
+  /// launched in this workspace.
   ///
-  /// Is `null` if the PATH for child processes is the same as the PATH for the
-  /// current process.
+  /// Is `null` if the [EnvironmentVariableKey.path] for child processes is the
+  /// same as the [EnvironmentVariableKey.path] for the current process.
   late final String? childProcessPath = sdkPath == null
       ? null
       : utils.addToPathEnvVar(
           directory: p.join(sdkPath!, 'bin'),
-          currentPath: currentPlatform.environment['PATH']!,
+          currentPath:
+              currentPlatform.environment[EnvironmentVariableKey.path]!,
           // We prepend the path to the bin directory in the Dart/Flutter SDK
           // because we want to shadow any system wide SDK.
           prepend: true,
@@ -180,9 +185,10 @@ class MelosWorkspace {
   /// Execute a command in the root of this workspace.
   Future<int> exec(List<String> execArgs, {bool onlyOutputOnError = false}) {
     final environment = {
-      'MELOS_ROOT_PATH': path,
-      if (sdkPath != null) utils.envKeyMelosSdkPath: sdkPath!,
-      if (childProcessPath != null) 'PATH': childProcessPath!,
+      EnvironmentVariableKey.melosRootPath: path,
+      if (sdkPath != null) EnvironmentVariableKey.melosSdkPath: sdkPath!,
+      if (childProcessPath != null)
+        EnvironmentVariableKey.path: childProcessPath!,
     };
 
     return utils.startCommand(

diff --git a/packages/melos/test/commands/run_test.dart b/packages/melos/test/commands/run_test.dart
@@ -1,4 +1,5 @@
 import 'package:melos/melos.dart';
+import 'package:melos/src/common/environment_variable_key.dart';
 import 'package:melos/src/common/glob.dart';
 import 'package:melos/src/common/io.dart';
 import 'package:melos/src/common/platform.dart';
@@ -90,7 +91,8 @@ melos run test_script
     );
 
     test(
-      'merges filters from `packageFilters` and `$envKeyMelosPackages`',
+      'merges filters from `packageFilters` and '
+      '`${EnvironmentVariableKey.melosPackages}`',
       withMockPlatform(
         () async {
           final workspaceDir = await createTemporaryWorkspace(
@@ -170,7 +172,7 @@ melos run test_script
           );
         },
         platform: FakePlatform.fromPlatform(const LocalPlatform())
-          ..environment[envKeyMelosPackages] = 'b,c',
+          ..environment[EnvironmentVariableKey.melosPackages] = 'b,c',
       ),
     );
 

diff --git a/packages/melos/test/workspace_test.dart b/packages/melos/test/workspace_test.dart
@@ -19,6 +19,7 @@ import 'dart:io';
 import 'dart:io' as io;
 
 import 'package:melos/melos.dart';
+import 'package:melos/src/common/environment_variable_key.dart';
 import 'package:melos/src/common/glob.dart';
 import 'package:melos/src/common/utils.dart';
 import 'package:melos/src/workspace.dart';
@@ -210,11 +211,12 @@ The packages that caused the problem are:
             expect(workspace.sdkPath, '/sdks/env');
           },
           platform: FakePlatform.fromPlatform(const LocalPlatform())
-            ..environment[envKeyMelosSdkPath] = '/sdks/env',
+            ..environment[EnvironmentVariableKey.melosSdkPath] = '/sdks/env',
         );
       });
 
-      test('prepend SDK bin directory to PATH', () async {
+      test('prepend SDK bin directory to ${EnvironmentVariableKey.path}',
+          () async {
         withMockPlatform(
           () {
             final workspace = VirtualWorkspaceBuilder(
@@ -224,7 +226,7 @@ The packages that caused the problem are:
             expect(workspace.path, '/sdk$pathEnvVarSeparator/bin');
           },
           platform: FakePlatform.fromPlatform(const LocalPlatform())
-            ..environment['PATH'] = '/bin',
+            ..environment[EnvironmentVariableKey.path] = '/bin',
         );
       });
     });