fix(init.js): do not run git init unnecessarily

The tool depends on git for managing the vendored libdragon copy. It used to run `git init` to make
sure there is a git repos initialized. This works if the p;roject root matched the git troot. This
was mostly the case but when there is an NPM root we were attaching to it to be npm project
compatible and if that root didin't match the git root, this was cresulting in an extra git root.

re #69

modules/actions/destroy.js

@@ -1,7 +1,7 @@
 const fs = require('fs/promises');
 const path = require('path');
 
-const { destroyContainer } = require('./utils');
+const { destroyContainer } = require('../utils');
 const { CONFIG_FILE, LIBDRAGON_PROJECT_MANIFEST } = require('../constants');
 const { fileExists, dirExists, log, ValidationError } = require('../helpers');
 const chalk = require('chalk');

modules/actions/exec.js

@@ -13,8 +13,8 @@ const {
 } = require('../helpers');
 
 const { start } = require('./start');
-const { dockerHostUserParams } = require('./docker-utils');
-const { installDependencies } = require('./utils');
+const { dockerHostUserParams } = require('../docker-utils');
+const { installDependencies } = require('../utils');
 
 /**
  * @param {import('../project-info').LibdragonInfo} libdragonInfo

modules/actions/init.js

@@ -10,7 +10,8 @@ const {
   updateImage,
   runGitMaybeHost,
   destroyContainer,
-} = require('./utils');
+  ensureGit,
+} = require('../utils');
 
 const {
   LIBDRAGON_PROJECT_MANIFEST,
@@ -109,9 +110,8 @@ const autoVendor = async (info) => {
   }
 
   // Container re-init breaks file modes assume there is git for this case.
-  // TODO: we should remove the unnecessary inits in the future.
   if (!process.env.DOCKER_CONTAINER) {
-    await runGitMaybeHost(info, ['init']);
+    await ensureGit(info);
   }
 
   // TODO: TS thinks this is already defined
@@ -172,6 +172,8 @@ const autoVendor = async (info) => {
       // This will throw if git user name/email is not set up. Let's not assume
       // anything for now. This means subtree is not supported for someone without
       // git on the host machine.
+      // TODO: this is probably creating an unnecessary commit for someone who
+      // just created a git repo and knows what they are doing.
       await runGitMaybeHost(info, [
         'commit',
         '--allow-empty',
@@ -262,6 +264,7 @@ async function init(info) {
     // We have created a new container, save the new info ASAP
     // When in a container, we should already have git
     // Re-initing breaks file modes anyways
+    // TODO: if this fails, we leave the project in a bad state
     await initGitAndCacheContainerId(
       /** @type Parameters<initGitAndCacheContainerId>[0] */ (info)
     );

modules/actions/install.js

@@ -1,4 +1,4 @@
-const { installDependencies } = require('./utils');
+const { installDependencies } = require('../utils');
 const { start } = require('./start');
 
 /**

modules/actions/start.js

@@ -14,7 +14,7 @@ const {
   checkContainerAndClean,
   checkContainerRunning,
   destroyContainer,
-} = require('./utils');
+} = require('../utils');
 
 /**
  * Create a new container

modules/actions/stop.js

@@ -1,6 +1,6 @@
 const { spawnProcess, ValidationError } = require('../helpers');
 
-const { checkContainerRunning } = require('./utils');
+const { checkContainerRunning } = require('../utils');
 
 /**
  * @param {import('../project-info').LibdragonInfo} libdragonInfo

modules/actions/update.js

@@ -5,7 +5,7 @@ const {
   installDependencies,
   updateImage,
   destroyContainer,
-} = require('./utils');
+} = require('../utils');
 const { start } = require('./start');
 
 /**

modules/actions/docker-utils.js → modules/docker-utils.js

@@ -1,6 +1,6 @@
 /**
  *
- * @param {import('../project-info').LibdragonInfo} libdragonInfo
+ * @param {import('./project-info').LibdragonInfo} libdragonInfo
  */
 function dockerHostUserParams(libdragonInfo) {
   const { uid, gid } = libdragonInfo.userInfo;

modules/actions/npm-utils.js → modules/npm-utils.js

@@ -5,14 +5,14 @@ const _ = require('lodash');
 
 const { dockerHostUserParams } = require('./docker-utils');
 
-const { CONTAINER_TARGET_PATH } = require('../constants');
+const { CONTAINER_TARGET_PATH } = require('./constants');
 const {
   fileExists,
   toPosixPath,
   spawnProcess,
   dockerExec,
   ValidationError,
-} = require('../helpers');
+} = require('./helpers');
 
 async function findNPMRoot() {
   try {
@@ -29,7 +29,7 @@ async function findNPMRoot() {
 
 /**
  * Install other NPM dependencies if this is an NPM project
- * @param {import('../project-info').LibdragonInfo} libdragonInfo
+ * @param {import('./project-info').LibdragonInfo} libdragonInfo
  */
 const installNPMDependencies = async (libdragonInfo) => {
   const npmRoot = await findNPMRoot();

modules/project-info.js

@@ -5,9 +5,9 @@ const fs = require('fs/promises');
 const {
   checkContainerAndClean,
   initGitAndCacheContainerId,
-} = require('./actions/utils');
+} = require('./utils');
 
-const { findNPMRoot } = require('./actions/npm-utils');
+const { findNPMRoot } = require('./npm-utils');
 
 const {
   LIBDRAGON_PROJECT_MANIFEST,
@@ -105,8 +105,7 @@ async function findContainerId(libdragonInfo) {
     if (longId.length === 64) {
       // This shouldn't happen but if the user somehow deleted the .git folder
       // (we don't have the container id file at this point) we can recover the
-      // project. `git init` is safe anyways and it is not executed if strategy
-      // is `manual`
+      // project. This is safe and it is not executed if strategy is `manual`
       await initGitAndCacheContainerId({
         ...libdragonInfo,
         containerId: longId,

modules/actions/utils.js → modules/utils.js

@@ -1,10 +1,7 @@
 const path = require('path');
 const fs = require('fs/promises');
 
-const {
-  CONTAINER_TARGET_PATH,
-  CACHED_CONTAINER_FILE,
-} = require('../constants');
+const { CONTAINER_TARGET_PATH, CACHED_CONTAINER_FILE } = require('./constants');
 
 const {
   fileExists,
@@ -16,13 +13,13 @@ const {
   CommandError,
   ValidationError,
   toNativePath,
-} = require('../helpers');
+} = require('./helpers');
 
 const { dockerHostUserParams } = require('./docker-utils');
 const { installNPMDependencies } = require('./npm-utils');
 
 /**
- * @param {import('../project-info').LibdragonInfo} libdragonInfo
+ * @param {import('./project-info').LibdragonInfo} libdragonInfo
  */
 const installDependencies = async (libdragonInfo) => {
   const buildScriptPath = path.join(
@@ -54,7 +51,7 @@ const installDependencies = async (libdragonInfo) => {
 /**
  * Downloads the given docker image. Returns false if the local image is the
  * same, true otherwise.
- * @param {import('../project-info').LibdragonInfo} libdragonInfo
+ * @param {import('./project-info').LibdragonInfo} libdragonInfo
  * @param {string} newImageName
  */
 const updateImage = async (libdragonInfo, newImageName) => {
@@ -98,7 +95,7 @@ const updateImage = async (libdragonInfo, newImageName) => {
 };
 
 /**
- * @param {import('../project-info').LibdragonInfo} libdragonInfo
+ * @param {import('./project-info').LibdragonInfo} libdragonInfo
  */
 const destroyContainer = async (libdragonInfo) => {
   assert(
@@ -128,9 +125,9 @@ const destroyContainer = async (libdragonInfo) => {
 
 /**
  *
- * @param {import('../project-info').LibdragonInfo} libdragonInfo
+ * @param {import('./project-info').LibdragonInfo} libdragonInfo
  * @param {string[]} params
- * @param {import('../helpers').SpawnOptions} options
+ * @param {import('./helpers').SpawnOptions} options
  */
 async function runGitMaybeHost(libdragonInfo, params, options = {}) {
   assert(
@@ -139,9 +136,30 @@ async function runGitMaybeHost(libdragonInfo, params, options = {}) {
   );
   try {
     const isWin = /^win/.test(process.platform);
+
+    // When using host's git the actual top level can be higher in the tree
+    // rather that at the root of the project. We need to find it to run some
+    // git operations like subtree.
+    const gitRoot = (
+      await spawnProcess(
+        'git',
+        ['-C', libdragonInfo.root, 'rev-parse', '--show-toplevel'],
+        { inheritStdin: false }
+      ).catch(() => {
+        // Either this is not a git repo or the host does not have git
+        // We might also screw something up but we can live with it for now
+        return '';
+      })
+    ).trim();
+
     return await spawnProcess(
       'git',
-      ['-C', libdragonInfo.root, ...params],
+      [
+        '-C',
+        libdragonInfo.root,
+        ...(gitRoot ? ['--git-dir', `${gitRoot}/.git`] : []),
+        ...params,
+      ],
       // Windows git is breaking the TTY somehow - disable TTY for now
       // We are not able to display progress for the initial clone b/c of this
       // Enable progress otherwise.
@@ -171,7 +189,7 @@ async function runGitMaybeHost(libdragonInfo, params, options = {}) {
 }
 
 /**
- * @param {import('../project-info').LibdragonInfo} libdragonInfo
+ * @param {import('./project-info').LibdragonInfo} libdragonInfo
  */
 async function checkContainerAndClean(libdragonInfo) {
   assert(
@@ -229,17 +247,16 @@ async function checkContainerRunning(containerId) {
 }
 
 /**
- * @param {import('../project-info').LibdragonInfo & {containerId: string}} libdragonInfo
+ * @param {import('./project-info').LibdragonInfo & {containerId: string}} libdragonInfo
  */
 async function initGitAndCacheContainerId(libdragonInfo) {
   if (!libdragonInfo.containerId) {
     return;
   }
 
-  // If there is managed vendoring, make sure we have a git repo. `git init` is
-  // safe anyways...
+  // If there is managed vendoring, make sure we have a git repo.
   if (libdragonInfo.vendorStrategy !== 'manual') {
-    await runGitMaybeHost(libdragonInfo, ['init']);
+    await ensureGit(libdragonInfo);
   }
 
   const rootGitFolder = (
@@ -263,6 +280,29 @@ async function initGitAndCacheContainerId(libdragonInfo) {
   }
 }
 
+/**
+ * Makes sure there is a parent git repository. If not, it will create one at
+ * project root.
+ * @param {import('./project-info').LibdragonInfo} info
+ */
+async function ensureGit(info) {
+  const gitRoot = (
+    await runGitMaybeHost(info, ['rev-parse', '--show-toplevel']).catch(() => {
+      // Probably host does not have git, can ignore
+      return '';
+    })
+  ).trim();
+
+  // If the host does not have git installed, this will not run unless we
+  // have already initialized it via the container, in which case we would
+  // have it as the git root. This is not expected to mess with host git flows
+  // where there is a git working tree higher in the host filesystem, which
+  // the container does not have access to.
+  if (!gitRoot) {
+    await runGitMaybeHost(info, ['init']);
+  }
+}
+
 module.exports = {
   installDependencies,
   updateImage,
@@ -271,4 +311,5 @@ module.exports = {
   checkContainerAndClean,
   initGitAndCacheContainerId,
   runGitMaybeHost,
+  ensureGit,
 };