Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[common-utils] - append shell history functionality - #1026 #1157

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/common-utils/devcontainer-feature.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@
"type": "boolean",
"default": false,
"description": "Add packages from non-free Debian repository? (Debian only)"
},
"allowShellHistory": {
"type": "boolean",
"default": false,
"description": "Preserve shell history across dev container instances? (Currently supports bash, zsh, and fish)"
}
}
}
},
"mounts": [{
"source": "devcontainers",
"target": "/devcontainers",
"type": "volume"
}]
}
48 changes: 48 additions & 0 deletions src/common-utils/main.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ USERNAME="${USERNAME:-"automatic"}"
USER_UID="${USERUID:-"automatic"}"
USER_GID="${USERGID:-"automatic"}"
ADD_NON_FREE_PACKAGES="${NONFREEPACKAGES:-"false"}"
ALLOW_SHELL_HISTORY="${ALLOWSHELLHISTORY:-"false"}"

MARKER_FILE="/usr/local/etc/vscode-dev-containers/common"

Expand Down Expand Up @@ -567,6 +568,53 @@ if [ "${INSTALL_ZSH}" = "true" ]; then
fi
fi

# *********************************
# ** Enable shell history **
# *********************************

if [ "${ALLOW_SHELL_HISTORY}" = "true" ]; then
echo "Activating feature 'shell-history'"
echo "User: ${USERNAME} User home: ${user_home}"

if ! command -v uuidgen &> /dev/null; then
sudo apt-get update
sudo apt-get install -y uuid-runtime
fi
# Create the shell history directory in the mounted volume
DEVCONTAINER_ID=$(uuidgen)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One downside of generating a UUID here is that it will change with each rebuild of the container image. The ${devcontainerId} value could be passed into the container with "containerEnv" (it could make sense for the CLI to do this automatically, but it currently doesn't) where this script could use it when run as part of the "postCreateCommand". 🤔 (It would also make sense for the CLI to pass DEVCONTAINER_ID into the feature install scripts.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made changes as suggested.

HISTORY_DIR="/devcontainers/${DEVCONTAINER_ID}/shellHistory"
USER_HISTORY_FILE="${user_home}/.bash_history"
VOLUME_HISTORY_FILE="${HISTORY_DIR}/.bash_history"

# Create the history directory in the volume, if it doesn’t already exist
sudo mkdir -p "${HISTORY_DIR}"
sudo chown -R "${USERNAME}" "${HISTORY_DIR}"
sudo chmod -R u+rwx "${HISTORY_DIR}"

# Ensure the volume's history file exists and set permissions
if [[ ! -f "${VOLUME_HISTORY_FILE}" ]]; then
# Create an empty history file if it doesn’t already exist
sudo touch "${VOLUME_HISTORY_FILE}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this won't work when the volume already exists because the volume is not mounted at this point and the image contents at /devcontainer will be copied to the volume only once - when the volume is created.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the check

sudo chown -R "${USERNAME}" "${VOLUME_HISTORY_FILE}"
sudo chmod -R u+rwx "${VOLUME_HISTORY_FILE}"
fi

# Create or update the user’s .bash_history to append to the volume’s history
if [[ ! -f "${USER_HISTORY_FILE}" ]]; then
sudo touch "${USER_HISTORY_FILE}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be overwritten by the symbolic link below. I think this can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

sudo chown -R "${USERNAME}" "${USER_HISTORY_FILE}"
sudo chmod -R u+rwx "${USER_HISTORY_FILE}"
fi

# Symlink for Bash history
sudo ln -sf ${USER_HISTORY_FILE} ${VOLUME_HISTORY_FILE}

# Configure immediate history saving to the volume
echo 'PROMPT_COMMAND="history -a; history -c; history -r"' >> "${user_home}/.bashrc"

echo "Shell history setup for persistent appending is complete."
fi

# *********************************
# ** Ensure config directory **
# *********************************
Expand Down
17 changes: 17 additions & 0 deletions test/common-utils/allow_shell_history.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/bin/bash

set -e

# Optional: Import test library
source dev-container-features-test-lib

# Definition specific tests
check "default-shell-is-zsh" bash -c "getent passwd $(whoami) | awk -F: '{ print $7 }' | grep '/bin/zsh'"
# check it overrides the ~/.zshrc with default dev containers template
check "default-zshrc-is-dev-container-template" bash -c "cat ~/.zshrc | grep ZSH_THEME | grep devcontainers"
check "zsh-path-contains-local-bin" zsh -l -c "echo $PATH | grep '/home/devcontainer/.local/bin'"

check "Ensure .zprofile is owned by remoteUser" bash -c "stat -c '%U' /home/devcontainer/.zprofile | grep devcontainer"

# Report result
reportResults
10 changes: 10 additions & 0 deletions test/common-utils/scenarios.json
Original file line number Diff line number Diff line change
Expand Up @@ -292,5 +292,15 @@
"features": {
"common-utils": {}
}
},
"allow_shell_history": {
"image": "debian:bookworm",
"features": {
"common-utils": {
"installZsh": true,
"allowShellHistory": true
}
},
"remoteUser": "vscode"
}
}
Loading