]> git.wincent.com - wincent.git/commitdiff
refactor: prepare to implement chown etc
authorGreg Hurrell <greg@hurrell.net>
Wed, 1 Apr 2020 17:37:38 +0000 (19:37 +0200)
committerGreg Hurrell <greg@hurrell.net>
Wed, 1 Apr 2020 17:38:04 +0000 (19:38 +0200)
src/Fig/compare.ts
src/Fig/operations/template.ts
src/Fig/task.ts
src/chown.ts [new file with mode: 0644]
src/stat.ts
src/tempfile.ts [new file with mode: 0644]

index 481d0ecd3fa40217cd0c80206f88c5740b6f8869..5e00c11a077b8d7e30bd611ea548c67c94ed29e9 100644 (file)
@@ -1,9 +1,13 @@
+// TODO: move a lot of the stuff that is currently under "Fig/" out of it
+// (original intent was to have a separation between generic stuff and
+// configuration-framework-specific entities. But in practice, the use of global
+// state and the amount of coupling we have between different modules means we
+// may as well consider them all to be equal citizens.
 import {promises as fs} from 'fs';
 import {dirname} from 'path';
 
 import ErrorWithMetadata from '../ErrorWithMetadata';
-
-import type {Stats} from 'fs';
+import stat from '../stat';
 
 // TODO: decide whether the Ansible definition of "force" (which we use below)
 // is the one that we want to actual stick with.
@@ -64,7 +68,7 @@ export default async function compare({
 }: Compare) {
   const diff: Diff = {path};
 
-  const stats = await lstat(path);
+  const stats = await stat(path);
 
   // BUG: if you specify "owner": "root", we should be able to manage files that
   // only root can stat, but this code stats as an unprivileged user
@@ -82,7 +86,7 @@ export default async function compare({
       // it can be created), and one of its parents not existing (in which case
       // we have to bail).
       const parent = dirname(path);
-      const stats = await lstat(parent);
+      const stats = await stat(parent);
       if (stats instanceof Error) {
         // Unlikely (we were able to stat object but not its parent).
         diff.error = stats;
@@ -104,13 +108,13 @@ export default async function compare({
 
   // Object exists.
   if (state === 'file') {
-    if (stats.isFile()) {
+    if (stats.type === 'file') {
       // Want "file", have "file": no state change required.
-    } else if (stats.isSymbolicLink()) {
+    } else if (stats.type === 'link') {
       // Going to have to overwrite symlink.
       diff.force = true;
       diff.state = 'file';
-    } else if (stats.isDirectory()) {
+    } else if (stats.type === 'directory') {
       diff.error = new ErrorWithMetadata(
         `Cannot replace directory ${stringify(path)} with file`
       );
@@ -133,15 +137,15 @@ export default async function compare({
       }
     }
   } else if (state === 'directory') {
-    if (stats.isDirectory()) {
+    if (stats.type === 'directory') {
       // Want "directory", have "directory": no state change required.
-    } else if (stats.isFile() || stats.isSymbolicLink()) {
+    } else if (stats.type === 'file' || stats.type === 'link') {
       if (force) {
         // Will have to remove file/link before creating directory.
         diff.force = true;
         diff.state = state;
       } else {
-        const entity = stats.isFile() ? 'file' : 'symbolic link';
+        const entity = stats.type === 'file' ? 'file' : 'symbolic link';
 
         diff.error = new ErrorWithMetadata(
           `Cannot replace ${entity} ${stringify(
@@ -167,22 +171,3 @@ export default async function compare({
 
   return diff;
 }
-
-/**
- * Wrapper for `fs.lstat` that returns a `Stats` object when `path` exists and is
- * accessible, `null` when the path does not exist, and an `Error` otherwise.
- */
-async function lstat(path: string): Promise<Stats | Error | null> {
-  try {
-    return await fs.lstat(path);
-  } catch (error) {
-    if (error.code === 'ENOENT') {
-      return null;
-    } else if (error.code === 'EACCES') {
-      // "permission denied"
-      return error;
-    } else {
-      return error;
-    }
-  }
-}
index d4b38c0e6f3f265115dad648e98af16bfa1c929d..04c81d544706852f8aae0ac3f0a645c2004ee565 100644 (file)
@@ -4,7 +4,7 @@ import ErrorWithMetadata from '../../ErrorWithMetadata';
 import {log} from '../../console';
 import expand from '../../expand';
 import run from '../../run';
-import stat from '../../stat';
+import tempfile from '../../tempfile';
 import {compile, fill} from '../../template';
 import Context from '../Context';
 import compare from '../compare';
@@ -28,8 +28,6 @@ export default async function template({
 }): Promise<void> {
   const target = expand(path);
 
-  log.info(`template ${src} -> ${target}`);
-
   const contents = (await Context.compile(src)).fill({variables});
 
   const diff = await compare({
@@ -42,8 +40,6 @@ export default async function template({
     state: 'file',
   });
 
-  console.log(diff);
-
   if (owner && owner !== Context.attributes.username) {
     log.notice(`needs sudo: ${Context.attributes.username} -> ${owner}`);
     const passphrase = await Context.sudoPassphrase;
@@ -55,25 +51,15 @@ export default async function template({
         error: result.error?.toString() ?? null,
       });
     }
-
-    // chown in node works with numeric uid and gid
   } else {
-    // open, write, mode
-    // can't chown, i think? without uid and gid
-
-    // TODO extract this somewhere else
-    // need low-level filesystem ops that are consumed by the high-level
-    // user-accessible ops
-    let current;
-
-    if (fs.existsSync(target)) {
-      current = fs.readFileSync(target, 'utf8');
+    if (diff.contents) {
+      log.info('change!');
+      const temp = await tempfile(contents);
 
-      if (current !== contents) {
-        log.info('change!');
-      } else {
-        log.info('no change');
-      }
+      // TODO: cp from temp to target
+      // TODO: deal with group/owner/mode etc
+    } else {
+      Context.informOk(`template ${path}`);
     }
   }
 }
index 2635f49ebbf9ad7ed983e32877c65d0188cedb77..0d09a460334ddbcc8e386b48f2a3f6432f66bcfe 100644 (file)
@@ -18,9 +18,9 @@ export default function task(name: string, callback: () => Promise<void>) {
   }
 
   assertAspect(aspect);
-  // TODO: use `caller` to make namespaced task name.
+
+  // TODO: we use `caller` to make namespaced task name.
   // (will be useful for --start-at)
   // also, we can make an interactive mode that lets us choose where to start
-
-  Context.tasks.register(aspect, callback, name);
+  Context.tasks.register(aspect, callback, `${aspect} | ${name}`);
 }
diff --git a/src/chown.ts b/src/chown.ts
new file mode 100644 (file)
index 0000000..1a1598c
--- /dev/null
@@ -0,0 +1,51 @@
+import ErrorWithMetadata from './ErrorWithMetadata';
+import Context from './Fig/Context';
+import run from './run';
+
+type Options = {
+  group?: string;
+  sudo?: boolean;
+  user?: string;
+};
+
+export default async function chown(
+  path: string,
+  options: Options = {}
+): Promise<void> {
+  if (Context.attributes.platform === 'darwin') {
+  } else {
+    throw new Error('TODO: implement');
+  }
+}
+
+// TODO: decide whether to throw/catch or just return errors
+
+/*
+
+let result;
+
+try {
+  result = await chown(path, options);
+
+  // code that needs result (either here...)
+} catch (error) {
+  return null; // or re-throw
+}
+
+// (or...) code that needs result
+
+// vs
+
+const result = await chown(path, options);
+
+if (result instanceof Error) {
+  return null; // or re-throw etc
+}
+
+// code that needs result...
+
+// less nesting, possibly clearer control flow, less fighting against block
+// scope etc.
+// catch still might be better though if you ever need to use finally etc.
+
+*/
index 7b3569ad8c6390825aa615086f96d5f8f85d5b74..87be50927f1c32719cde6363dcd5ec29c3d7f340 100644 (file)
@@ -26,7 +26,9 @@ const TYPE_MAP = {
  * on it to stat root-owned files; we need to be able to run a separate too,
  * with `sudo` if necessary.
  */
-export default async function stat(path: string): Promise<Stats> {
+export default async function stat(
+  path: string
+): Promise<Error | Stats | null> {
   if (Context.attributes.platform === 'darwin') {
     const formats = {
       group: '%Sg',
@@ -73,7 +75,9 @@ export default async function stat(path: string): Promise<Stats> {
         };
       }
 
-      if (!/permission denied/i.test(stderr)) {
+      if (/no such file/i.test(stderr)) {
+        return null;
+      } else if (!/permission denied/i.test(stderr)) {
         // Give up...
         break;
       }
diff --git a/src/tempfile.ts b/src/tempfile.ts
new file mode 100644 (file)
index 0000000..b122c3d
--- /dev/null
@@ -0,0 +1,25 @@
+import {randomBytes} from 'crypto';
+import {promises as fs} from 'fs';
+import {tmpdir} from 'os';
+import {join} from 'path';
+
+import {log} from './console';
+
+const TMP_DIR = tmpdir();
+
+/**
+ * Writes the `contents` to a temporary file and returns the path to that file.
+ * We rely on the operating system to clean-up such files (eventually), but
+ * leave them on the disk for debugging purposes.
+ */
+export default async function tempfile(contents: string): Promise<string> {
+  const name = randomBytes(16).toString('hex');
+
+  const path = join(TMP_DIR, name);
+
+  await fs.writeFile(path, contents, 'utf8');
+
+  log.debug(`Wrote ${contents.length} bytes to ${path}`);
+
+  return path;
+}