Skip to content

Conversation

@Wenzel
Copy link
Contributor

@Wenzel Wenzel commented May 19, 2025

No description provided.

@Wenzel Wenzel marked this pull request as ready for review July 29, 2025 15:21
@Wenzel Wenzel requested a review from Copilot July 29, 2025 15:21
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new tsffs-dev Docker target to support development workflows. The changes include refactoring the existing Dockerfile to use multi-stage builds with named stages and adding development container configuration.

  • Adds a multi-stage Docker build with tsffs-base, tsffs-dev, and tsffs-prod targets
  • Creates a development environment with user management, Rust toolchain, and proper permissions
  • Configures VS Code dev container integration with extensions

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
Dockerfile Adds multi-stage build with tsffs-dev target for development environment
.dockerignore Excludes dev container config and packages directory from build context
.devcontainer.json Configures VS Code dev container to use tsffs-dev target

Dockerfile Outdated
&& echo "$USERNAME ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/$USERNAME

# create group for developers
groupadd dev
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The 'dev' group is created but the user is not added to it. Consider adding the user to the group with 'usermod -a -G dev $USERNAME' to ensure proper permissions for /workspace/{simics,projects}.

Suggested change
groupadd dev
groupadd dev
# add $USERNAME to the dev group
usermod -a -G dev $USERNAME

Copilot uses AI. Check for mistakes.
chown -R root:dev /workspace/{simics,projects} && chmod -R g+w /workspace/{simics,projects}

# install Rust nightly for the user
sudo -E -u $USERNAME bash -c 'curl https://sh.rustup.rs -sSf | bash -s -- -y --default-toolchain none'
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

Installing Rust via curl piped to bash poses security risks. Consider using package manager installation or verifying the script's integrity with checksums.

Copilot uses AI. Check for mistakes.
# create group for developers
groupadd dev
# set /workspace/simics permissions to root:dev
chown -R root:dev /workspace/{simics,projects} && chmod -R g+w /workspace/{simics,projects}
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

[nitpick] Setting group write permissions recursively on /workspace/{simics,projects} may be overly permissive. Consider limiting write access to specific subdirectories that need modification.

Suggested change
chown -R root:dev /workspace/{simics,projects} && chmod -R g+w /workspace/{simics,projects}
chown -R root:dev /workspace/simics/specific-subdir /workspace/projects/specific-subdir && \
chmod -R g+w /workspace/simics/specific-subdir /workspace/projects/specific-subdir

Copilot uses AI. Check for mistakes.
@Wenzel Wenzel force-pushed the dockerfile_dev branch 4 times, most recently from 5ca2e5c to 30ba116 Compare July 29, 2025 19:51
@Wenzel Wenzel merged commit 5ff86b6 into intel:main Aug 4, 2025
17 checks passed
@Wenzel Wenzel deleted the dockerfile_dev branch August 4, 2025 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant