From ac9c80b9fa9edc1db5b58f64912add85fbfb369c Mon Sep 17 00:00:00 2001 From: Ali Naci Erdem Date: Sun, 13 Nov 2022 14:37:15 +0300 Subject: [PATCH] feat: add support for running inside a container The cli can now work properly inside a container if the `DOCKER_CONTAINER` environment variable is set. A devcontainer is set up for this repository both as an example and as a development environment for the cli and libdragon itself. --- .devcontainer/Dockerfile | 9 ++++++ .devcontainer/devcontainer.json | 9 ++++++ .devcontainer/init.sh | 9 ++++++ .gitattributes | 1 + .gitmodules | 2 +- README.md | 25 +++++++++++++++- modules/actions/destroy.js | 8 ++++- modules/actions/exec.js | 18 ++++++++++++ modules/actions/init.js | 43 ++++++++++++++++----------- modules/actions/install.js | 33 +++++++++++---------- modules/actions/start.js | 23 ++++++++++++++- modules/actions/stop.js | 8 ++++- modules/actions/update-and-start.js | 9 +++++- modules/actions/utils.js | 34 ++++++++++++++++++++++ modules/helpers.js | 45 +++++++++++++++++++++++------ modules/project-info.js | 13 +++++++-- 16 files changed, 240 insertions(+), 49 deletions(-) create mode 100644 .devcontainer/Dockerfile create mode 100644 .devcontainer/devcontainer.json create mode 100644 .devcontainer/init.sh create mode 100644 .gitattributes diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile new file mode 100644 index 0000000..2305687 --- /dev/null +++ b/.devcontainer/Dockerfile @@ -0,0 +1,9 @@ +# syntax=docker/dockerfile:1 +FROM ghcr.io/dragonminded/libdragon:latest + +ENV DOCKER_CONTAINER=true + +COPY ./.devcontainer/init.sh /tmp/init.sh + +WORKDIR /tmp +RUN /bin/bash -c ./init.sh \ No newline at end of file diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 0000000..4122f2c --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,9 @@ +{ + // Use image instead of build, if you don't need the cli installed + // "image": ghcr.io/dragonminded/libdragon:latest + "build": { "dockerfile": "./Dockerfile", "context": "../" }, + "workspaceMount": "source=${localWorkspaceFolder},target=/libdragon,type=bind", + "workspaceFolder": "/libdragon", + // You can execute ./build.sh instead, if you don't need the cli in the container + "postCreateCommand": "npm install && npm link && libdragon install" +} \ No newline at end of file diff --git a/.devcontainer/init.sh b/.devcontainer/init.sh new file mode 100644 index 0000000..dc75ae3 --- /dev/null +++ b/.devcontainer/init.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash +set -euo pipefail +IFS=$'\n\t' + +# Install nodejs & npm +apt-get update +apt install curl -y +curl -fsSL https://deb.nodesource.com/setup_16.x | bash - +apt-get install nodejs \ No newline at end of file diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..94f480d --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +* text=auto eol=lf \ No newline at end of file diff --git a/.gitmodules b/.gitmodules index 1a22c15..be69963 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ -[submodule "libdragon"] +[submodule "libdragon"] path = libdragon url = https://github.com/DragonMinded/libdragon branch = trunk diff --git a/README.md b/README.md index 244fc49..2af0ae3 100644 --- a/README.md +++ b/README.md @@ -184,7 +184,7 @@ For a quick development loop it really helps linking the code in this repository npm link -in the root of the repository. Once you do this, running `libdragon` will use the code here rather than the actual npm installation. Then you can test your changes in the libdragon project here or elsewhere on your computer. +in the root of the repository. Once you do this, running `libdragon` will use the code here rather than the actual npm installation. Then you can test your changes in the libdragon project here or elsewhere on your computer. This setup is automatically done if you use the [devcontainer](#experimental-devcontainer-support). When you are happy with your changes, you can verify you conform to the coding standards via: @@ -201,6 +201,29 @@ This repository uses [`semantic-release`](https://github.com/semantic-release/se It will create a `semantic-release` compatible commit from your current staged changes. +### Experimental devcontainer support + +The repository provides a configuration (in `.devcontainer`) so that IDEs that support it can create and run the Docker container for you. Then, you can start working on it as if you are working on a machine with libdragon installed. + +With the provided setup, you can continue using the cli in the container and it will work for non-container specific actions like `install`, `disasm` etc. You don't have to use the cli in the container, but you can. In general it will be easier and faster to just run `make` in the container but this setup is included to ease developing the cli as well. + +To create your own dev container backed project, you can use the contents of the `.devcontainer` folder as reference. You don't need to include nodejs or the cli and you can just run `build.sh` as `postCreateCommand`. See the `devcontainer.json` for more details. As long as your container have the `DOCKER_CONTAINER` environment variable, the tool can work inside a container. + +#### Caveats + +- In the devcontainer, uploading via USB will not work. +- Error matching is not yet tested. +- Ideally the necessary extensions should be automatically installed. This is not configured yet. + +
+ vscode instructions + + - Make sure you have the [Dev container extension](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers) installed and you fulfill its [requirements](https://code.visualstudio.com/docs/devcontainers/containers). + - Clone this repository with `--recurse-submodules` or run `git submodule update --init`. + - Open command palette and run `Dev Containers: Reopen in container`. + - It will prepare the container and open it in the editor. +
+ ## As an NPM dependency You can install libdragon as an NPM dependency by `npm install libdragon --save` in order to use docker in your N64 projects. A `libdragon` command similar to global intallation is provided that can be used in your NPM scripts as follows; diff --git a/modules/actions/destroy.js b/modules/actions/destroy.js index 364c103..bcecb7d 100644 --- a/modules/actions/destroy.js +++ b/modules/actions/destroy.js @@ -3,13 +3,19 @@ const path = require('path'); const { destroyContainer } = require('./utils'); const { CONFIG_FILE, LIBDRAGON_PROJECT_MANIFEST } = require('../constants'); -const { fileExists, dirExists, log } = require('../helpers'); +const { fileExists, dirExists, log, ValidationError } = require('../helpers'); const chalk = require('chalk'); /** * @param {import('../project-info').LibdragonInfo} libdragonInfo */ const destroy = async (libdragonInfo) => { + if (process.env.DOCKER_CONTAINER) { + throw new ValidationError( + `Not possible to destroy the container from inside.` + ); + } + await destroyContainer(libdragonInfo); const projectPath = path.join(libdragonInfo.root, LIBDRAGON_PROJECT_MANIFEST); diff --git a/modules/actions/exec.js b/modules/actions/exec.js index 28df713..6c82932 100644 --- a/modules/actions/exec.js +++ b/modules/actions/exec.js @@ -9,6 +9,7 @@ const { fileExists, dirExists, CommandError, + spawnProcess, } = require('../helpers'); const { start } = require('./start'); @@ -45,6 +46,23 @@ const exec = async (info) => { true ); + // Don't even bother here, we are already in a container. + if (process.env.DOCKER_CONTAINER) { + const enableTTY = Boolean(process.stdout.isTTY && process.stdin.isTTY); + await spawnProcess(info.options.EXTRA_PARAMS[0], parameters, { + userCommand: true, + // Inherit stdin/out in tandem if we are going to disable TTY o/w the input + // stream remains inherited by the node process while the output pipe is + // waiting data from stdout and it behaves like we are still controlling + // the spawned process while the terminal is actually displaying say for + // example `less`. + inheritStdout: enableTTY, + inheritStdin: enableTTY, + inheritStderr: true, + }); + return info; + } + const stdin = new PassThrough(); /** @type {string[]} */ diff --git a/modules/actions/init.js b/modules/actions/init.js index a001c28..cbca562 100644 --- a/modules/actions/init.js +++ b/modules/actions/init.js @@ -108,7 +108,11 @@ const autoVendor = async (info) => { return info; } - await runGitMaybeHost(info, ['init']); + // 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']); + } // TODO: TS thinks this is already defined const detectedStrategy = await autoDetect(info); @@ -220,28 +224,33 @@ async function init(info) { LIBDRAGON_PROJECT_MANIFEST )} exists. This is already a libdragon project, starting it...` ); - if (info.options.DOCKER_IMAGE) { - info = await syncImageAndStart(info); - } else { - info = { - ...info, - containerId: await start(info), - }; + if (!process.env.DOCKER_CONTAINER) { + if (info.options.DOCKER_IMAGE) { + info = await syncImageAndStart(info); + } else { + info = { + ...info, + containerId: await start(info), + }; + } } + info = await autoVendor(info); await installDependencies(info); return info; } - await updateImage(info, info.imageName); - - // Download image and start it - info.containerId = await start(info); - - // We have created a new container, save the new info ASAP - await initGitAndCacheContainerId( - /** @type Parameters[0] */ (info) - ); + if (!process.env.DOCKER_CONTAINER) { + // Download image and start it + await updateImage(info, info.imageName); + info.containerId = await start(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 + await initGitAndCacheContainerId( + /** @type Parameters[0] */ (info) + ); + } info = await autoVendor(info); diff --git a/modules/actions/install.js b/modules/actions/install.js index 331094c..7e70fde 100644 --- a/modules/actions/install.js +++ b/modules/actions/install.js @@ -13,21 +13,24 @@ const { log } = require('../helpers'); */ const install = async (libdragonInfo) => { let updatedInfo = libdragonInfo; - const imageName = libdragonInfo.options.DOCKER_IMAGE; - // If an image is provided, attempt to install - if (imageName) { - log( - chalk.yellow( - 'Using `install` action to update the docker image is deprecated. Use the `update` action instead.' - ) - ); - updatedInfo = await syncImageAndStart(libdragonInfo); - } else { - // Make sure existing one is running - updatedInfo = { - ...updatedInfo, - containerId: await start(libdragonInfo), - }; + + if (!process.env.DOCKER_CONTAINER) { + const imageName = libdragonInfo.options.DOCKER_IMAGE; + // If an image is provided, attempt to install + if (imageName) { + log( + chalk.yellow( + 'Using `install` action to update the docker image is deprecated. Use the `update` action instead.' + ) + ); + updatedInfo = await syncImageAndStart(libdragonInfo); + } else { + // Make sure existing one is running + updatedInfo = { + ...updatedInfo, + containerId: await start(libdragonInfo), + }; + } } // Re-install vendors on new image diff --git a/modules/actions/start.js b/modules/actions/start.js index d84d581..204efe9 100644 --- a/modules/actions/start.js +++ b/modules/actions/start.js @@ -1,7 +1,14 @@ const chalk = require('chalk').stderr; const { CONTAINER_TARGET_PATH } = require('../constants'); -const { spawnProcess, log, print, dockerExec } = require('../helpers'); +const { + spawnProcess, + log, + print, + dockerExec, + assert, + ValidationError, +} = require('../helpers'); const { checkContainerAndClean, @@ -14,6 +21,11 @@ const { * @param {import('../project-info').LibdragonInfo} libdragonInfo */ const initContainer = async (libdragonInfo) => { + assert( + !process.env.DOCKER_CONTAINER, + new Error('initContainer does not make sense in a container') + ); + let newId; try { log('Creating new container...'); @@ -79,6 +91,11 @@ const initContainer = async (libdragonInfo) => { * @param {import('../project-info').LibdragonInfo} libdragonInfo */ const start = async (libdragonInfo) => { + assert( + !process.env.DOCKER_CONTAINER, + new Error('Cannot start a container when we are already in a container.') + ); + const running = libdragonInfo.containerId && (await checkContainerRunning(libdragonInfo.containerId)); @@ -108,6 +125,10 @@ module.exports = /** @type {const} */ ({ * @param {import('../project-info').LibdragonInfo} libdragonInfo */ fn: async (libdragonInfo) => { + if (process.env.DOCKER_CONTAINER) { + throw new ValidationError(`We are already in a container.`); + } + const containerId = await start(libdragonInfo); print(containerId); return { ...libdragonInfo, containerId }; diff --git a/modules/actions/stop.js b/modules/actions/stop.js index 47614d7..37aea81 100644 --- a/modules/actions/stop.js +++ b/modules/actions/stop.js @@ -1,4 +1,4 @@ -const { spawnProcess } = require('../helpers'); +const { spawnProcess, ValidationError } = require('../helpers'); const { checkContainerRunning } = require('./utils'); @@ -7,6 +7,12 @@ const { checkContainerRunning } = require('./utils'); * @returns {Promise} */ const stop = async (libdragonInfo) => { + if (process.env.DOCKER_CONTAINER) { + throw new ValidationError( + `Not possible to stop the container from inside.` + ); + } + const running = libdragonInfo.containerId && (await checkContainerRunning(libdragonInfo.containerId)); diff --git a/modules/actions/update-and-start.js b/modules/actions/update-and-start.js index 22983e9..d56b16b 100644 --- a/modules/actions/update-and-start.js +++ b/modules/actions/update-and-start.js @@ -1,4 +1,4 @@ -const { log } = require('../helpers'); +const { log, assert } = require('../helpers'); const { updateImage, destroyContainer } = require('./utils'); const { start } = require('./start'); @@ -6,6 +6,13 @@ const { start } = require('./start'); * @param {import('../project-info').LibdragonInfo} libdragonInfo */ async function syncImageAndStart(libdragonInfo) { + assert( + !process.env.DOCKER_CONTAINER, + new Error( + '[syncImageAndStart] We should already know we are in a container.' + ) + ); + const oldImageName = libdragonInfo.imageName; const imageName = libdragonInfo.options.DOCKER_IMAGE ?? oldImageName; // If an image is provided, always attempt to install it diff --git a/modules/actions/utils.js b/modules/actions/utils.js index 28e6ba1..7ec794e 100644 --- a/modules/actions/utils.js +++ b/modules/actions/utils.js @@ -58,6 +58,11 @@ const installDependencies = async (libdragonInfo) => { * @param {string} newImageName */ const updateImage = async (libdragonInfo, newImageName) => { + assert( + !process.env.DOCKER_CONTAINER, + new Error('[updateImage] should not be called in a container') + ); + // Will not take too much time if already have the same const download = async () => { log(`Downloading docker image: ${newImageName}`); @@ -96,6 +101,11 @@ const updateImage = async (libdragonInfo, newImageName) => { * @param {import('../project-info').LibdragonInfo} libdragonInfo */ const destroyContainer = async (libdragonInfo) => { + assert( + !process.env.DOCKER_CONTAINER, + new Error('[destroyContainer] should not be called in a container') + ); + if (libdragonInfo.containerId) { await spawnProcess('docker', [ 'container', @@ -144,6 +154,11 @@ async function runGitMaybeHost(libdragonInfo, params, options = {}) { 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 @@ -159,6 +174,13 @@ async function runGitMaybeHost(libdragonInfo, params, options = {}) { * @param {import('../project-info').LibdragonInfo} libdragonInfo */ async function checkContainerAndClean(libdragonInfo) { + assert( + !process.env.DOCKER_CONTAINER, + new Error( + '[checkContainerAndClean] We should already know we are in a container.' + ) + ); + const id = libdragonInfo.containerId && ( @@ -188,6 +210,13 @@ async function checkContainerAndClean(libdragonInfo) { * @param {string} containerId */ async function checkContainerRunning(containerId) { + assert( + !process.env.DOCKER_CONTAINER, + new Error( + '[checkContainerRunning] We should already know we are in a container.' + ) + ); + const running = ( await spawnProcess('docker', [ 'container', @@ -203,11 +232,16 @@ async function checkContainerRunning(containerId) { * @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 (libdragonInfo.vendorStrategy !== 'manual') { await runGitMaybeHost(libdragonInfo, ['init']); } + const gitFolder = path.join(libdragonInfo.root, '.git'); if (await dirExists(gitFolder)) { await fs.writeFile( diff --git a/modules/helpers.js b/modules/helpers.js index 07e1e4a..1a855a1 100644 --- a/modules/helpers.js +++ b/modules/helpers.js @@ -157,6 +157,10 @@ function spawnProcess( spawnOptions: {}, } ) { + assert( + cmd !== 'docker' || !process.env.DOCKER_CONTAINER, + new Error('Trying to invoke docker inside a container.') + ); return new Promise((resolve, reject) => { /** @type {Buffer[]} */ const stdout = []; @@ -273,11 +277,6 @@ const dockerExec = /** @type {DockerExec} */ ( /** @type {SpawnOptions | undefined} */ options ) { - assert( - !!libdragonInfo.containerId, - new Error('Trying to invoke dockerExec without a containerId.') - ); - // TODO: assert for invalid args const haveDockerParams = Array.isArray(dockerParams) && Array.isArray(cmdWithParams); @@ -286,6 +285,35 @@ const dockerExec = /** @type {DockerExec} */ ( options = /** @type {SpawnOptions} */ (cmdWithParams); } + const finalCmdWithParams = haveDockerParams ? cmdWithParams : dockerParams; + const finalDockerParams = haveDockerParams ? dockerParams : []; + + assert( + finalDockerParams.findIndex( + (val) => val === '--workdir=' || val === '-w=' + ) === -1, + new Error('Do not use `=` syntax when setting working dir') + ); + + // Convert docker execs into regular commands in the correct cwd + if (process.env.DOCKER_CONTAINER) { + const workDirIndex = finalDockerParams.findIndex( + (val) => val === '--workdir' || val === '-w' + ); + const workDir = + workDirIndex >= 0 ? finalDockerParams[workDirIndex + 1] : undefined; + return spawnProcess(finalCmdWithParams[0], finalCmdWithParams.slice(1), { + ...options, + spawnOptions: { cwd: workDir, ...options?.spawnOptions }, + }); + } + + assert( + !!libdragonInfo.containerId, + new Error('Trying to invoke dockerExec without a containerId.') + ); + + /** @type string[] */ const additionalParams = []; // Docker TTY wants in & out streams both to be a TTY @@ -309,11 +337,10 @@ const dockerExec = /** @type {DockerExec} */ ( 'docker', [ 'exec', - ...(haveDockerParams - ? [...dockerParams, ...additionalParams] - : additionalParams), + ...finalDockerParams, + ...additionalParams, libdragonInfo.containerId, - ...(haveDockerParams ? cmdWithParams : dockerParams), + ...finalCmdWithParams, ], options ); diff --git a/modules/project-info.js b/modules/project-info.js index c9aa41f..b7c54fe 100644 --- a/modules/project-info.js +++ b/modules/project-info.js @@ -64,6 +64,11 @@ const { * @param {LibdragonInfo} libdragonInfo */ async function findContainerId(libdragonInfo) { + assert( + !process.env.DOCKER_CONTAINER, + new Error('[findContainerId] We should already know we are in a container.') + ); + const idFile = path.join(libdragonInfo.root, '.git', CACHED_CONTAINER_FILE); if (await fileExists(idFile)) { const id = (await fs.readFile(idFile, { encoding: 'utf8' })).trim(); @@ -225,15 +230,19 @@ const readProjectInfo = async function (optionInfo) { } } - info.containerId = await findContainerId(info); - log(`Active container id: ${info.containerId}`, true); + if (!process.env.DOCKER_CONTAINER) { + info.containerId = await findContainerId(info); + log(`Active container id: ${info.containerId}`, true); + } // For imageName, flag has the highest priority followed by the one read from // the file and then if there is any matching container, name is read from it. // As last option fallback to default value. // If still have the container, read the image name from it + // No need to do anything if we are in a container if ( + !process.env.DOCKER_CONTAINER && !info.imageName && info.containerId && (await checkContainerAndClean(info))