fix: correct handling of paths on Windows and those containing spaces (…

…#223)

Fixes #222

.github/workflows/build.yaml

@@ -1,5 +1,7 @@
 #
-# Builds and tests the packages on Linux.
+# Builds and tests the packages.  This builds on Linux by default, but
+# can be configured to build on Windows and/or macOS by uncommenting
+# the matrix config lines below.
 #
 # This runs after changes are pushed to a feature branch.  It does not
 # run for pushes to the main branch because that is covered by the
@@ -15,8 +17,22 @@ on:
 
 jobs:
   build:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.config.os }}
+    timeout-minutes: 20
+    strategy:
+      matrix:
+        config:
+          - { plat: 'linux', os: 'ubuntu-20.04' }
+          # - { plat: 'mac', os: 'macos-10.15' }
+          # - { plat: 'win', os: 'windows-2019' }
     steps:
+      # Force Unix-style line endings (otherwise Prettier checks will fail on Windows)
+      - name: Configure git to use Unix-style line endings
+        if: matrix.config.plat == 'win'
+        run: |
+          git config --global core.autocrlf false
+          git config --global core.eol lf
+
       - name: Check out repo
         uses: actions/checkout@v2
 
@@ -32,8 +48,15 @@ jobs:
         with:
           version: 7
 
+      - name: Configure pnpm
+        if: matrix.config.plat == 'win'
+        run: |
+          # Force pnpm to use bash shell for running scripts on Windows
+          pnpm config set script-shell bash
+
       - name: Get pnpm store directory
         id: pnpm-cache
+        shell: bash
         run: echo "::set-output name=pnpm_cache_dir::$(pnpm store path)"
 
       - name: Enable pnpm cache
@@ -45,9 +68,11 @@ jobs:
             pnpm-store-${{ runner.os }}-
 
       - name: Install dependencies
+        shell: bash
         run: |
           pnpm install
 
       - name: Build and test
+        shell: bash
         run: |
           ./scripts/ci-build

packages/build/src/build/impl/gen-model.ts

@@ -26,9 +26,10 @@ export async function generateModel(context: BuildContext, plugins: Plugin[]): P
   const config = context.config
   const prepDir = config.prepDir
 
-  // XXX: On Windows, we need to use Windows-specific commands; need to revisit
-  const isWin = process.platform === 'win32'
-  const sdeCmdPath = isWin ? `${config.sdeCmdPath}.cmd` : config.sdeCmdPath
+  // TODO: For now we assume the path is to the `main.js` file in the cli package;
+  // this seems to work on both Unix and Windows, but we may need to revisit this
+  // as part of removing the `sdeCmdPath` config hack
+  const sdeCmdPath = config.sdeCmdPath
 
   // Process the mdl file(s)
   for (const plugin of plugins) {

packages/build/src/config/config-loader.ts

@@ -1,8 +1,8 @@
 // Copyright (c) 2022 Climate Interactive / New Venture Fund
 
 import { existsSync, lstatSync, mkdirSync } from 'fs'
-import { join as joinPath, resolve as resolvePath } from 'path'
-import { pathToFileURL } from 'url'
+import { dirname, join as joinPath, relative, resolve as resolvePath } from 'path'
+import { fileURLToPath } from 'url'
 
 import type { Result } from 'neverthrow'
 import { err, ok } from 'neverthrow'
@@ -40,10 +40,9 @@ export async function loadConfig(
     // Use the given `UserConfig` object
     userConfig = config
   } else {
-    // Load the project-specific config.  Note that on Windows the import path
-    // must be a `file://` URL, so we have to convert here.  If no `--config`
-    // arg was specified on the command line, look for a `sde.config.js` file
-    // in the current directory, and failing that, use a default config.
+    // Load the project-specific config.  If no `--config` arg was specified
+    // on the command line, look for a `sde.config.js` file in the current
+    // directory, and failing that, use a default config.
     // TODO: Create a default config if no file found; for now, we just fail
     let configPath: string
     if (typeof config === 'string') {
@@ -55,8 +54,8 @@ export async function loadConfig(
       if (!existsSync(configPath)) {
         return err(new Error(`Cannot find config file '${configPath}'`))
       }
-      const configUrl = pathToFileURL(configPath).toString()
-      const configModule = await import(configUrl)
+      const configRelPath = relativeToSourcePath(configPath)
+      const configModule = await import(configRelPath)
       userConfig = await configModule.config()
     } catch (e) {
       return err(new Error(`Failed to load config file '${configPath}': ${e.message}`))
@@ -169,3 +168,16 @@ function resolveUserConfig(
     sdeCmdPath
   }
 }
+
+/**
+ * Return a Unix-style path (e.g. '../../foo.js') that is relative to the directory of
+ * the current source file.  This can be used to construct a path that is safe for
+ * dynamic import on either Unix or Windows.
+ *
+ * @param filePath The path to make relative.
+ */
+function relativeToSourcePath(filePath: string): string {
+  const srcDir = dirname(fileURLToPath(import.meta.url))
+  const relPath = relative(srcDir, filePath)
+  return relPath.replaceAll('\\', '/')
+}

packages/cli/package.json

@@ -22,7 +22,6 @@
     "@sdeverywhere/compile": "^0.7.0",
     "bufx": "^1.0.5",
     "byline": "^5.0.0",
-    "fs-extra": "^10.1.0",
     "ramda": "^0.27.0",
     "shelljs": "^0.8.3",
     "yargs": "^17.5.1"

packages/cli/src/main.js

@@ -26,6 +26,7 @@ import fs from 'fs'
 import path from 'path'
 
 import yargs from 'yargs'
+
 import sdeBundle from './sde-bundle.js'
 import sdeDev from './sde-dev.js'
 import sdeGenerate from './sde-generate.js'
@@ -41,12 +42,14 @@ import sdeTest from './sde-test.js'
 import sdeNames from './sde-names.js'
 import sdeCauses from './sde-causes.js'
 import sdeWhich from './sde-which.js'
+import { parentDirForFileUrl } from './utils.js'
 
 // Workaround yargs issue where it doesn't find version from package.json
 // automatically in all cases in ESM context
-const srcDir = new URL('.', import.meta.url).pathname
+const srcDir = parentDirForFileUrl(import.meta.url)
 const pkgFile = path.resolve(srcDir, '..', 'package.json')
 const pkg = JSON.parse(fs.readFileSync(pkgFile))
+const pkgVersion = pkg.version
 
 const yarg = yargs(process.argv.slice(2))
 yarg
@@ -70,7 +73,7 @@ yarg
   .command(sdeWhich)
   .demandCommand(1)
   .help()
-  .version(pkg.version)
+  .version(pkgVersion)
   .alias('h', 'help')
   .alias('v', 'version')
   .wrap(yarg.terminalWidth())

packages/cli/src/sde-bundle.js

@@ -4,6 +4,8 @@ import path from 'path'
 
 import { build as runBuild } from '@sdeverywhere/build'
 
+import { parentDirForFileUrl } from './utils.js'
+
 export let command = 'bundle [options]'
 export let describe = 'build and bundle a model as specified by a config file'
 export let builder = {
@@ -25,7 +27,7 @@ export let bundle = async (configPath, verbose) => {
   if (verbose) {
     logLevels.push('verbose')
   }
-  const srcDir = new URL('.', import.meta.url).pathname
+  const srcDir = parentDirForFileUrl(import.meta.url)
   const sdeDir = path.resolve(srcDir, '..')
   const sdeCmdPath = path.resolve(srcDir, 'main.js')
   const result = await runBuild('production', {

packages/cli/src/sde-compile.js

@@ -1,8 +1,8 @@
-import fs from 'fs-extra'
+import fs from 'fs'
 import path from 'path'
 import sh from 'shelljs'
 
-import { buildDir, execCmd, modelPathProps } from './utils.js'
+import { buildDir, execCmd, modelPathProps, parentDirForFileUrl } from './utils.js'
 
 export let command = 'compile [options] <model>'
 export let describe = 'compile the generated model to an executable file'
@@ -43,15 +43,16 @@ export default {
 }
 
 let linkCSourceFiles = (modelDirname, buildDirname) => {
-  let cDirname = path.join(new URL('.', import.meta.url).pathname, 'c')
+  let srcDir = parentDirForFileUrl(import.meta.url)
+  let cDirname = path.join(srcDir, 'c')
   sh.ls(cDirname).forEach(filename => {
-    // If a C source file is present in the model directory, link to it instead
-    // as an override.
+    // If a C source file is present in the model directory, copy that one into
+    // the build directory to override the one from `src/c`.
     let srcPathname = path.join(modelDirname, filename)
     if (!fs.existsSync(srcPathname)) {
       srcPathname = path.join(cDirname, filename)
     }
     let dstPathname = path.join(buildDirname, filename)
-    fs.ensureSymlinkSync(srcPathname, dstPathname)
+    fs.copyFileSync(srcPathname, dstPathname)
   })
 }

packages/cli/src/sde-dev.js

@@ -4,6 +4,8 @@ import path from 'path'
 
 import { build as runBuild } from '@sdeverywhere/build'
 
+import { parentDirForFileUrl } from './utils.js'
+
 export let command = 'dev [options]'
 export let describe = 'run a model in a live development environment'
 export let builder = {
@@ -25,7 +27,7 @@ export let dev = async (configPath, verbose) => {
   if (verbose) {
     logLevels.push('verbose')
   }
-  const srcDir = new URL('.', import.meta.url).pathname
+  const srcDir = parentDirForFileUrl(import.meta.url)
   const sdeDir = path.resolve(srcDir, '..')
   const sdeCmdPath = path.resolve(srcDir, 'main.js')
   const result = await runBuild('development', {

packages/cli/src/sde-which.js

@@ -1,5 +1,7 @@
 import path from 'path'
 
+import { parentDirForFileUrl } from './utils.js'
+
 let command = 'which'
 let describe = 'print the SDEverywhere home directory'
 let builder = {}
@@ -8,7 +10,8 @@ let handler = argv => {
 }
 let which = () => {
   // The SDEverywhere home directory is one level above the src directory where this code runs.
-  let homeDir = path.resolve(new URL('..', import.meta.url).pathname)
+  let srcDir = parentDirForFileUrl(import.meta.url)
+  let homeDir = path.resolve(srcDir, '..')
   console.log(homeDir)
 }
 export default {

packages/cli/src/utils.js

@@ -1,5 +1,6 @@
 import fs from 'fs'
 import path from 'path'
+import { fileURLToPath } from 'url'
 
 import B from 'bufx'
 import R from 'ramda'
@@ -77,6 +78,20 @@ export function parseSpec(specFilename) {
   return spec
 }
 
+/**
+ * Return the absolute path to the directory in which the given (source)
+ * file is located.  (This is a replacement for `__dirname`, which is not
+ * available in an ESM context.)
+ *
+ * @param srcFileUrl The URL for the source file (i.e., `import.meta.url`).
+ */
+export function parentDirForFileUrl(srcFileUrl) {
+  return path.dirname(fileURLToPath(srcFileUrl))
+}
+
+/**
+ * Ensure the output directory exists for a given output file path.
+ */
 export function outputDir(outfile, modelDirname) {
   if (outfile) {
     outfile = path.dirname(outfile)

packages/plugin-check/src/plugin.ts

@@ -1,7 +1,7 @@
 // Copyright (c) 2022 Climate Interactive / New Venture Fund
 
-import { join as joinPath } from 'path'
-import { pathToFileURL } from 'url'
+import { dirname, join as joinPath, relative } from 'path'
+import { fileURLToPath } from 'url'
 
 import type { InlineConfig, ViteDevServer } from 'vite'
 import { build, createServer } from 'vite'
@@ -108,9 +108,8 @@ class CheckPlugin implements Plugin {
     context.log('info', 'Running model checks...')
 
     // Load the bundles used by the model check/compare configuration.  We
-    // always initialize the "current" bundle.   Note that on Windows the
-    // dynamic import path must be a `file://` URL, so we have to convert.
-    const moduleR = await import(pathToFileURL(testOptions.currentBundlePath).toString())
+    // always initialize the "current" bundle.
+    const moduleR = await import(relativeToSourcePath(testOptions.currentBundlePath))
     const bundleR = moduleR.createBundle() as Bundle
     const nameR = testOptions.currentBundleName
 
@@ -121,7 +120,7 @@ class CheckPlugin implements Plugin {
     let bundleL: Bundle
     let nameL: string
     if (this.options?.baseline) {
-      const moduleL = await import(pathToFileURL(this.options.baseline.path).toString())
+      const moduleL = await import(relativeToSourcePath(this.options.baseline.path))
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       const rawBundleL: any = moduleL.createBundle() as any
       if (rawBundleL.version === bundleR.version) {
@@ -131,7 +130,7 @@ class CheckPlugin implements Plugin {
     }
 
     // Get the model check/compare configuration
-    const testConfigModule = await import(pathToFileURL(testOptions.testConfigPath).toString())
+    const testConfigModule = await import(relativeToSourcePath(testOptions.testConfigPath))
     const checkOptions = await testConfigModule.getConfigOptions(bundleL, bundleR, {
       nameL,
       nameR
@@ -195,3 +194,16 @@ class CheckPlugin implements Plugin {
     )
   }
 }
+
+/**
+ * Return a Unix-style path (e.g. '../../foo.js') that is relative to the directory of
+ * the current source file.  This can be used to construct a path that is safe for
+ * dynamic import on either Unix or Windows.
+ *
+ * @param filePath The path to make relative.
+ */
+function relativeToSourcePath(filePath: string): string {
+  const srcDir = dirname(fileURLToPath(import.meta.url))
+  const relPath = relative(srcDir, filePath)
+  return relPath.replaceAll('\\', '/')
+}

packages/plugin-check/src/vite-config-for-tests.ts

@@ -19,8 +19,11 @@ export function createViteConfigForTests(projDir: string, prepDir: string, mode:
   // directory where the glob is used).
   const templateSrcDir = resolvePath(root, 'src')
   const relProjDir = relative(templateSrcDir, projDir)
-  //  TODO: Use yamlPath from options
-  const yamlPath = `${relProjDir}/**/*.check.yaml`
+  // XXX: The glob pattern must use forward slashes only, so on Windows we need to
+  // convert backslashes to slashes
+  const relProjDirPath = relProjDir.replaceAll('\\', '/')
+  // TODO: Use yamlPath from options
+  const yamlPath = `${relProjDirPath}/**/*.check.yaml`
 
   // // Read the `package.json` for the template project
   // const pkgPath = resolvePath(root, 'package.json')