refactor: stop using container git

The tool used to fallback to container git if the host system didn't have it installed. Now it is strictly required. BREAKING CHANGE: Host system must have git installed after this update.
anacierdem · Oct 19, 2024 · ce4ae92 · ce4ae92
1 parent 5471392
commit ce4ae92
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 65 deletions.
diff --git a/README.md b/README.md
@@ -6,9 +6,7 @@ This is a wrapper for a docker container to make managing the libdragon toolchai
 
 ## Prerequisites
 
-You should have [docker](https://www.docker.com/products/docker-desktop) (`>= 27.2.0`) installed on your system.
-
-`git` is not strictly required to use the tool. Still, it is highly recommended to have git on your host machine as it will be used instead of the one in the container.
+You should have [docker](https://www.docker.com/products/docker-desktop) (`>= 27.2.0`) && [git](https://git-scm.com/downloads) installed on your system.
 
 ## Installation
 

diff --git a/modules/actions/init.js b/modules/actions/init.js
@@ -10,7 +10,7 @@ const {
   installDependencies,
   initGitAndCacheContainerId,
   updateImage,
-  runGitMaybeHost,
+  runGit,
   destroyContainer,
   ensureGit,
 } = require('../utils');
@@ -61,15 +61,11 @@ const autoDetect = async (info) => {
 
   if (
     vendorTargetExists &&
-    (await runGitMaybeHost(
-      info,
-      ['submodule', 'status', info.vendorDirectory],
-      {
-        inheritStdin: false,
-        inheritStdout: false,
-        inheritStderr: false,
-      }
-    ).catch((e) => {
+    (await runGit(info, ['submodule', 'status', info.vendorDirectory], {
+      inheritStdin: false,
+      inheritStdout: false,
+      inheritStderr: false,
+    }).catch((e) => {
       if (!(e instanceof CommandError)) {
         throw e;
       }
@@ -80,7 +76,7 @@ const autoDetect = async (info) => {
   }
 
   if (vendorTargetExists) {
-    const gitLogs = await runGitMaybeHost(info, ['log'], {
+    const gitLogs = await runGit(info, ['log'], {
       inheritStdin: false,
       inheritStdout: false,
       inheritStderr: false,
@@ -161,7 +157,7 @@ const autoVendor = async (info) => {
 
   if (info.vendorStrategy === 'submodule') {
     try {
-      await runGitMaybeHost(info, [
+      await runGit(info, [
         'submodule',
         'add',
         '--force',
@@ -188,7 +184,7 @@ const autoVendor = async (info) => {
   if (info.vendorStrategy === 'subtree') {
     // Create a commit if it does not exist. This is required for subtree.
     try {
-      await runGitMaybeHost(info, ['rev-parse', 'HEAD']);
+      await runGit(info, ['rev-parse', 'HEAD']);
     } catch (e) {
       if (!(e instanceof CommandError)) throw e;
 
@@ -197,16 +193,16 @@ const autoVendor = async (info) => {
       // 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, [
+      await runGit(info, [
         'commit',
         '--allow-empty',
         '-n',
         '-m',
-        'Initial commit.',
+        '"Initial commit."',
       ]);
     }
 
-    await runGitMaybeHost(info, [
+    await runGit(info, [
       'subtree',
       'add',
       '--prefix',
@@ -321,7 +317,7 @@ async function init(info) {
 
     if ((await autoDetect(info)) === 'submodule') {
       try {
-        const existingBranchName = await runGitMaybeHost(
+        const existingBranchName = await runGit(
           info,
           [
             '-C',

diff --git a/modules/actions/update.js b/modules/actions/update.js
@@ -1,7 +1,7 @@
 const { log, assert } = require('../helpers');
 const { LIBDRAGON_GIT } = require('../constants');
 const {
-  runGitMaybeHost,
+  runGit,
   installDependencies,
   updateImage,
   destroyContainer,
@@ -55,15 +55,15 @@ const update = async (info) => {
   }
 
   if (info.vendorStrategy === 'submodule') {
-    await runGitMaybeHost(info, [
+    await runGit(info, [
       'submodule',
       'update',
       '--remote',
       '--merge',
       info.vendorDirectory,
     ]);
   } else if (info.vendorStrategy === 'subtree') {
-    await runGitMaybeHost(info, [
+    await runGit(info, [
       'subtree',
       'pull',
       '--prefix',

diff --git a/modules/utils.js b/modules/utils.js
@@ -10,13 +10,11 @@ const {
   dockerExec,
   dirExists,
   assert,
-  CommandError,
   ValidationError,
   toNativePath,
 } = require('./helpers');
 
 const { dockerHostUserParams } = require('./docker-utils');
-const { installNPMDependencies } = require('./npm-utils');
 
 /**
  * @param {import('./project-info').LibdragonInfo} libdragonInfo
@@ -44,8 +42,6 @@ const installDependencies = async (libdragonInfo) => {
     ],
     ['/bin/bash', './build.sh']
   );
-
-  await installNPMDependencies(libdragonInfo);
 };
 
 /**
@@ -119,8 +115,7 @@ const destroyContainer = async (libdragonInfo) => {
 };
 
 /**
- * Invokes host git with provided params. If host does not have git, falls back
- * to the docker git, with the nix user set to the user running libdragon.
+ * Invokes host git with provided params.
  */
 
 /**
@@ -129,43 +124,23 @@ const destroyContainer = async (libdragonInfo) => {
  * @param {string[]} params
  * @param {import('./helpers').SpawnOptions} options
  */
-async function runGitMaybeHost(libdragonInfo, params, options = {}) {
+async function runGit(libdragonInfo, params, options = {}) {
   assert(
     libdragonInfo.vendorStrategy !== 'manual',
     new Error('Should never run git if vendoring strategy is manual.')
   );
-  try {
-    const isWin = /^win/.test(process.platform);
-
-    return await spawnProcess(
-      'git',
-      ['-C', libdragonInfo.root, ...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.
-      isWin
-        ? { inheritStdin: false, ...options }
-        : { inheritStdout: true, inheritStderr: true, ...options }
-    );
-  } catch (e) {
-    if (e instanceof CommandError) {
-      throw e;
-    }
-
-    assert(
-      !process.env.DOCKER_CONTAINER,
-      new Error('[runGitMaybeHost] Native git should exist in a container.')
-    );
-
-    return await dockerExec(
-      libdragonInfo,
-      // Use the host user when initializing git as we will need access
-      [...dockerHostUserParams(libdragonInfo)],
-      ['git', ...params],
-      // Let's enable tty here to show the progress
-      { inheritStdout: true, inheritStderr: true, ...options }
-    );
-  }
+  const isWin = /^win/.test(process.platform);
+
+  return await spawnProcess(
+    'git',
+    ['-C', libdragonInfo.root, ...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.
+    isWin
+      ? { inheritStdin: false, ...options }
+      : { inheritStdout: true, inheritStderr: true, ...options }
+  );
 }
 
 /**
@@ -267,7 +242,7 @@ async function initGitAndCacheContainerId(libdragonInfo) {
  */
 async function ensureGit(info) {
   const gitRoot = (
-    await runGitMaybeHost(info, ['rev-parse', '--show-toplevel'], {
+    await runGit(info, ['rev-parse', '--show-toplevel'], {
       inheritStdin: false,
       inheritStdout: false,
       inheritStderr: false,
@@ -283,7 +258,7 @@ async function ensureGit(info) {
   // 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']);
+    await runGit(info, ['init']);
   }
 }
 
@@ -294,6 +269,6 @@ module.exports = {
   checkContainerRunning,
   checkContainerAndClean,
   initGitAndCacheContainerId,
-  runGitMaybeHost,
+  runGit,
   ensureGit,
 };