Initial Eclipse S-CORE Development Container #1
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| { | ||
| "features": { | ||
| "ghcr.io/devcontainers-extra/features/pre-commit:2": { | ||
| "version": "2.0.18", | ||
| "resolved": "ghcr.io/devcontainers-extra/features/pre-commit@sha256:6e0bb2ce80caca1d94f44dab5d0653d88a1c00984e590adb7c6bce012d0ade6e", | ||
| "integrity": "sha256:6e0bb2ce80caca1d94f44dab5d0653d88a1c00984e590adb7c6bce012d0ade6e" | ||
| }, | ||
| "ghcr.io/devcontainers/features/docker-in-docker:2": { | ||
| "version": "2.12.2", | ||
| "resolved": "ghcr.io/devcontainers/features/docker-in-docker@sha256:842d2ed40827dc91b95ef727771e170b0e52272404f00dba063cee94eafac4bb", | ||
| "integrity": "sha256:842d2ed40827dc91b95ef727771e170b0e52272404f00dba063cee94eafac4bb" | ||
| }, | ||
| "ghcr.io/devcontainers/features/git-lfs:1": { | ||
| "version": "1.2.5", | ||
| "resolved": "ghcr.io/devcontainers/features/git-lfs@sha256:71c2b371cf12ab7fcec47cf17369c6f59156100dad9abf9e4c593049d789de72", | ||
| "integrity": "sha256:71c2b371cf12ab7fcec47cf17369c6f59156100dad9abf9e4c593049d789de72" | ||
| }, | ||
| "ghcr.io/devcontainers/features/git:1": { | ||
| "version": "1.3.4", | ||
| "resolved": "ghcr.io/devcontainers/features/git@sha256:f24645e64ad39a596131a50ec96f7d5cf7a2a87544cce772dd4b7182a233e98a", | ||
| "integrity": "sha256:f24645e64ad39a596131a50ec96f7d5cf7a2a87544cce772dd4b7182a233e98a" | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| { | ||
| "image": "mcr.microsoft.com/devcontainers/javascript-node:0-18", | ||
| "features": { | ||
| "ghcr.io/devcontainers/features/git:1": {}, | ||
| "ghcr.io/devcontainers/features/git-lfs:1": {}, | ||
| "ghcr.io/devcontainers/features/docker-in-docker:2": {}, | ||
| "ghcr.io/devcontainers-extra/features/pre-commit:2": { | ||
| "version": "4.2.0" | ||
| } | ||
| }, | ||
| "postCreateCommand": "${containerWorkspaceFolder}/.devcontainer/post_create_command.sh", | ||
| "customizations": { | ||
| "vscode": { | ||
| "extensions": [ | ||
| "mads-hartmann.bash-ide-vscode", | ||
| "dbaeumer.vscode-eslint", | ||
| "EditorConfig.EditorConfig" | ||
| ], | ||
| "settings": { | ||
| "files.insertFinalNewline": true | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| #!/usr/bin/env bash | ||
| npm install -g @devcontainers/cli | ||
| pre-commit install |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| * @opajonk @lurtz | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| name: 'Validate DevContainer' | ||
| description: 'This workflow is checking that updates do not break stuff. If on main branch, publish to latest tag.' | ||
| on: | ||
| pull_request: | ||
| push: | ||
| branches: | ||
| - main | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| build: | ||
| name: 'Check, Build, Test, Publish DevContainer' | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| packages: write | ||
| id-token: write | ||
|
|
||
| steps: | ||
| - name: Checkout (GitHub) | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - name: Login to GitHub Container Registry | ||
| uses: docker/login-action@v2 | ||
| with: | ||
| registry: ghcr.io | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.GHCR_TOKEN }} | ||
|
Comment on lines
+30
to
+31
Contributor
Author
There was a problem hiding this comment. This will need an update. We need a secret which is allowed to publish to the image registry. |
||
|
|
||
| - name: Check, Build, Test, Publish | ||
| uses: devcontainers/ci@v0.3 | ||
| with: | ||
| cacheFrom: ghcr.io/eclipse-score/devcontainer | ||
| imageName: ghcr.io/eclipse-score/devcontainer | ||
| # publish latest from main branch; tags are handled in release workflow | ||
| imageTag: latest | ||
| refFilterForPush: 'refs/heads/main' | ||
| runCmd: | | ||
| # Check | ||
| pre-commit run --show-diff-on-failure --color=always --all-files || exit -1 | ||
|
|
||
| # Build | ||
| ./scripts/build.sh | ||
|
|
||
| # Test | ||
| ./scripts/test.sh | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| name: 'Validate & Publish DevContainer' | ||
| description: 'This workflow is checking that for releases, updates do not break stuff and publishes the released container.' | ||
| on: | ||
| push: | ||
| tags: | ||
| - '[0-9]+.[0-9]+.[0-9]+' | ||
|
|
||
| jobs: | ||
| build: | ||
| name: 'Check, Build, Test, Publish DevContainer' | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| packages: write | ||
| id-token: write | ||
|
|
||
| steps: | ||
| - name: Checkout (GitHub) | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - name: Login to GitHub Container Registry | ||
| uses: docker/login-action@v2 | ||
| with: | ||
| registry: ghcr.io | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.GHCR_TOKEN }} | ||
|
Comment on lines
+25
to
+26
Contributor
Author
There was a problem hiding this comment. This will need an update. We need a secret which is allowed to publish to the image registry.
Contributor
There was a problem hiding this comment. When I test the .devcontainer at communication I get this error: Is this expected? This is the used config
Contributor
Author
There was a problem hiding this comment. Yes that will work only after the merge. The CI did not run in S-CORE context yet. |
||
|
|
||
| - name: Check, Build, Test, Publish | ||
| uses: devcontainers/ci@v0.3 | ||
| with: | ||
| imageName: ghcr.io/eclipse-score/devcontainer | ||
| cacheFrom: ghcr.io/eclipse-score/devcontainer | ||
| imageTag: ${{ github.ref_name }} | ||
| runCmd: | | ||
| # Check | ||
| pre-commit run --show-diff-on-failure --color=always --all-files || exit -1 | ||
|
|
||
| # Build | ||
| ./scripts/build.sh | ||
|
|
||
| # Test | ||
| ./scripts/test.sh | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # Exported image files shall never be committed. | ||
| /export.img |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| repos: | ||
| - repo: https://github.com/pre-commit/pre-commit-hooks | ||
| rev: cef0300fd0fc4d2a87a85fa2093c6b283ea36f4b # v5.0.0 | ||
| hooks: | ||
| - id: check-yaml | ||
| - id: end-of-file-fixer | ||
| exclude: "devcontainer-lock.json" | ||
| - id: trailing-whitespace | ||
| - id: check-shebang-scripts-are-executable | ||
| - id: check-executables-have-shebangs | ||
|
Comment on lines
+2
to
+10
There was a problem hiding this comment. Me personally would never force pre-commit anything. But if this helps your workflow & you want this in your repository, seems fine by me.
Contributor
Author
There was a problem hiding this comment. I would recommend this, yes. We have made very good experience with "strict automated checks, but open to changes". What I mean is: don't "overdocument" and "overchecklist" - rather have an automation do it. This is cheap and can handle really a lot of issues. But: be open if people have doubts about settings. It should be always possible to discuss and give arguments why things are set like they are. |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,132 @@ | ||
| # Common DevContainer for Eclipse S-CORE | ||
| This repository contains the common [development container](https://containers.dev) for [Eclipse S-CORE](https://github.com/eclipse-score). | ||
| It contains all tools required to develop (modify, build, ...) Eclipse S-CORE. | ||
| All tool version are well-defined, and all tools are pre-configured to work as expected for Eclipse S-CORE development. | ||
| The container is [pre-built](https://containers.dev/guide/prebuild) in GitHub Actions as part of this repository, tested, published, and ready for use. | ||
|
|
||
| Using the pre-built container in an Eclipse S-CORE repository is described in the [Usage](#usage) section. | ||
|
|
||
| Modifying the content of the container is explained in the [Development](#development) section. | ||
|
|
||
| ## Usage | ||
|
|
||
| > **NOTE:** There are several development environments which support development containers; most notably [Visual Studio Code](https://code.visualstudio.com), but also [IntelliJ IDEA](https://www.jetbrains.com/idea) and others. | ||
| > See [here](https://containers.dev/supporting) for a more complete list. | ||
| > In the following, we assume that [Visual Studio Code](https://code.visualstudio.com) and its Dev Containers extension is used. | ||
| The [Dev Containers extension homepage](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers) has a description how to get up to speed on Windows, macOS and Linux operating systems. | ||
| From here on, we assume that such a development container setup is installed and running. | ||
|
|
||
| ### First-Time Setup | ||
|
|
||
| Add a file called `.devcontainer/devcontainer.json` to your repository. | ||
| It should contain the following: | ||
|
|
||
| ````json | ||
| { | ||
| "name": "eclipse-s-core", | ||
| "image": "ghcr.io/eclipse-score/devcontainer:<version>", | ||
| "initializeCommand": "mkdir -p ${localEnv:HOME}/.cache/bazel" | ||
| } | ||
| ```` | ||
|
|
||
| The `<version>` must be a [valid, published release](https://github.com/eclipse-score/devcontainer/tags). | ||
| You can also use `latest` as `<version>` to automatically follow the `main` branch - but be aware that this can result in undesired updates. | ||
| The `initializeCommand` is required to ensure the default Bazel cache directory exists on your host system. | ||
|
|
||
| To start using the container, click the **Reopen in Container** button when prompted by Visual Studio Code: | ||
|
|
||
|  | ||
|
|
||
| Alternatively, you can press <kbd>Ctrl</kbd> + <kbd>Shift</kbd> + <kbd>p</kbd> and run from there "Dev Containers: Reopen in Container". | ||
|
|
||
| The first time you do this, the container will be downloaded. | ||
| This may take some time. | ||
| Afterwards, Visual Studio Code should show this in the lower left corner of your window: | ||
|
|
||
|  | ||
|
|
||
| ### Inside the Container | ||
|
|
||
| Open a Terminal, and - for example - type `bazel build ...` to execute the default build of the repository. | ||
|
|
||
| After you have build the code, create [compilation databases](https://clang.llvm.org/docs/JSONCompilationDatabase.html) via Visual Studio Code [Task](https://code.visualstudio.com/docs/debugtest/tasks): | ||
|
|
||
| - C++: <kbd>Ctrl</kbd> + <kbd>Shift</kbd> + <kbd>p</kbd> -> `Tasks: Run Task` -> `Update compile_commands.json` | ||
| - Rust: <kbd>Ctrl</kbd> + <kbd>Shift</kbd> + <kbd>p</kbd> -> `Tasks: Run Task` -> `Generate rust-project.json` | ||
|
|
||
| These databases are used by Visual Studio Code to support code navigation and auto-completion with the help of [language servers](https://microsoft.github.io/language-server-protocol/). | ||
|
|
||
| Congratulations, you are now a dev container enthusiast 😊. | ||
|
|
||
| ## Development | ||
|
|
||
| > **NOTE:** This is about the development *of the DevContainer*, not about development of Eclipse S-CORE *using* the DevContainer. | ||
|
|
||
| The [Eclipse S-CORE](https://github.com/eclipse-score) development container is developed using - a development container! | ||
| That means, the usage is similarly simple: | ||
|
|
||
| ```` | ||
| git clone https://github.com/eclipse-score/devcontainer.git | ||
| code devcontainer | ||
| ```` | ||
| and "Reopen in Container". | ||
|
|
||
| ### Repository Structure | ||
| Ordered by importance: | ||
|
|
||
| * `src/s-core-devcontainer/` contains the sources for the Eclipse S-CORE DevContainer. | ||
| It uses pre-existing [DevContainer features](https://containers.dev/implementors/features/) to provide some standard tools like Git, LLVM, and others. | ||
| In addition, it uses a so-called "local" feature (cf. `src/s-core-devcontainer/.devcontainer/s-core-local`) for the remaining tools and configuration. | ||
| * `scripts/` contains scripts to build and test the container. | ||
| * `.devcontainer/` contains the definition of the DevContainer for **this** repository, i.e. the "devcontainer devcontainer". | ||
| There should rarely be a need to modify this. | ||
| * `.github/` contains the regular GitHub setup, with code owners and CI. | ||
| * `resources/` contains a few screenshots. | ||
|
|
||
| ### Modify, Build, Test, Use | ||
|
|
||
| It is very simple to develop the development container. | ||
| You can change files related to the container and then simply run the `scripts/*`. | ||
| They are used by the CI, but especially the build and test scripts can be run also locally out of the box: | ||
| ````console | ||
| $ ./scripts/build.sh | ||
| [... build output..] | ||
| {"outcome":"success","imageName":["vsc-s-core-devcontainer-209943ec6ff795f57b20cdf85a70c904d1e3b4a329d1e01c79f0ffea615c6e40-features"]} | ||
|
|
||
| $ ./scripts/test.sh | ||
| [... test output...] | ||
| 💯 All passed! | ||
| ```` | ||
| You can now also use this freshly built development container locally on your machine, e.g. to test the container as part of an Eclipse S-CORE module. | ||
| For this you must understand that you have the following situation: | ||
| ``` | ||
| +---------------------------------+ | ||
| | Development Container A | | ||
| | +---------------------------+ | | ||
| | | S-CORE DevContainer image | | | ||
| | +---------------------------+ | | ||
| +---------------------------------+ | ||
| ``` | ||
| `Development Container A` is the one you are running right now to develop the `S-CORE DevContainer` . | ||
| So in order to execute `S-CORE DevContainer` on your host (and test it as part of an S-CORE module), you need to | ||
|
|
||
| * export this newly built S-CORE DevContainer image | ||
| * import the image on your host machine | ||
| * use the image name in the `.devcontainer/devcontainer.json` of the targeted S-CORE module | ||
|
|
||
| Concretely, this can be done as follows: | ||
|
|
||
| * Run `docker save <imageName> > export.img` in `Development Container A`. | ||
| For example, given above build output, this would be `docker save vsc-s-core-devcontainer-209943ec6ff795f57b20cdf85a70c904d1e3b4a329d1e01c79f0ffea615c6e40-features > export.img` | ||
| * On your **host machine** (!!), open a console and run `docker load < /path/to/export.img`. | ||
| * In the working copy of the targeted S-CORE module, edit the file `.devcontainer/devcontainer.json` and change the `"image": "..."` entry to `"image": "<imageName>"`. | ||
| Given above build output, this would be `"image": "vsc-s-core-devcontainer-209943ec6ff795f57b20cdf85a70c904d1e3b4a329d1e01c79f0ffea615c6e40-features"`. | ||
| The Visual Studio Code instance related to the targeted S-CORE module will now ask you to rebuild the DevContainer. | ||
| Do so, and you have a running instance of `S-CORE DevContainer` related to the targeted S-CORE module. | ||
|
|
||
| ### Version Pinning | ||
|
|
||
| The `S-CORE DevContainer` pins feature and tool versions. | ||
| For tools that are not part of a trusted distribution (i.e. downloaded directly), it also pins the SHA256 hash. | ||
| While not being the most convenient choice for tool updates, this makes our supply chain much more secure. | ||
| Additionally, there are no "surprise updates" which unexpectedly break things. |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| #!/usr/bin/env bash | ||
| set -euxo pipefail | ||
|
|
||
| devcontainer build --workspace-folder src/s-core-devcontainer |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| #!/usr/bin/env bash | ||
| set -eo pipefail | ||
|
|
||
| if [[ -z ${HOME} ]]; then | ||
| HOME="/root" | ||
| fi | ||
|
|
||
| FAILED=() | ||
|
|
||
| echoStderr() | ||
| { | ||
| echo "$@" 1>&2 | ||
| } | ||
|
|
||
| check() { | ||
| LABEL=$1 | ||
| shift | ||
| echo -e "\n🧪 Testing ${LABEL}" | ||
| if "$@"; then | ||
| echo "✅ Passed!" | ||
| return 0 | ||
| else | ||
| echoStderr "❌ ${LABEL} check failed." | ||
| FAILED+=("${LABEL}") | ||
| return 1 | ||
| fi | ||
| } | ||
|
|
||
| reportResults() { | ||
| if [[ ${#FAILED[@]} -ne 0 ]]; then | ||
| echoStderr -e "\n💥 Failed tests:" "${FAILED[@]}" | ||
| exit 1 | ||
| else | ||
| echo -e "\n💯 All passed!" | ||
| exit 0 | ||
| fi | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| #!/usr/bin/env bash | ||
| set -euxo pipefail | ||
|
|
||
| IMAGE="s-core-devcontainer" | ||
|
|
||
| export DOCKER_BUILDKIT=1 | ||
|
|
||
| SCRIPT_PATH=$(readlink -f "$0") | ||
| SCRIPT_DIR=$(dirname -- "${SCRIPT_PATH}") | ||
| PROJECT_DIR=$(dirname -- "${SCRIPT_DIR}") | ||
| ID_LABEL="test-container=${IMAGE}" | ||
|
|
||
| devcontainer up \ | ||
| --id-label "${ID_LABEL}" \ | ||
| --workspace-folder "${PROJECT_DIR}/src/${IMAGE}/" \ | ||
| --remove-existing-container | ||
|
|
||
| CONTAINER_ID=$(docker container ls --filter "label=${ID_LABEL}" --quiet) | ||
| IMAGE_NAME=$(docker container inspect --format '{{ .Config.Image }}' "${CONTAINER_ID}") | ||
| IMAGE_ID=$(docker image ls --filter "reference=${IMAGE_NAME}" --quiet) | ||
|
|
||
| # Run actual test | ||
| echo "(*) Running test..." | ||
| devcontainer exec --workspace-folder "${PROJECT_DIR}/src/${IMAGE}" --id-label "${ID_LABEL}" \ | ||
| /bin/sh -c 'set -e && cd test-project && \ | ||
| ./test.sh' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| ARG VARIANT="noble" | ||
| FROM buildpack-deps:${VARIANT}-curl | ||
|
|
||
| LABEL dev.containers.features="common" | ||
|
|
||
| ARG VARIANT | ||
| RUN if [ "$VARIANT" = "noble" ]; then \ | ||
| if id "ubuntu" &>/dev/null; then \ | ||
| echo "Deleting user 'ubuntu' for $VARIANT" && userdel -f -r ubuntu || echo "Failed to delete ubuntu user for $VARIANT"; \ | ||
| else \ | ||
| echo "User 'ubuntu' does not exist for $VARIANT"; \ | ||
| fi; \ | ||
| fi |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This likely needs an update. Please suggest how this should look like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CODEOWNERS are the people that are required to approve a PR before it can be merged.
In this repository, it seems you two are the main protagonists, so for me this seems fine.