-
Notifications
You must be signed in to change notification settings - Fork 313
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
New environment configuration file: /etc/devcontainer #3593
Comments
Does |
I'll test it soon and let you know. Thanks. |
@chrmarti Yes, this is what I was looking for, but with a caveat. Without But with However, I was actually looking for an option And, I'm proposing to make
This would enable us to switch from: ARG PYTHON_VERSION="3.8.3"
ARG PYTHON_INSTALL_PATH="/usr/local/python${PYTHON_VERSION}"
ENV PIPX_HOME="/usr/local/py-utils"
ENV PATH="${PYTHON_INSTALL_PATH}/bin:${PATH}:${PIPX_HOME}/bin"
RUN bash -c "$(curl -fsSL "https://raw.githubusercontent.com/microsoft/vscode-dev-containers/master/script-library/python-debian.sh")" -- "${PYTHON_VERSION}" "${PYTHON_INSTALL_PATH}" "${PIPX_HOME}" to: RUN bash -c "$(curl -fsSL "https://raw.githubusercontent.com/microsoft/vscode-dev-containers/master/script-library/python-debian.sh")" |
Taking this as the feature request to add |
Why it is better than entry-point script? Container is not multitenant. Repetitive login can create sibling processes that will be never killed by init during the termination and become ghost OS processes. This is very dangerous. |
@chrmarti This is not about that, this is about setting up container wide environment not by using Did you check my example? With |
I don't understand how this is related to the entry point script. Remember that entry point script is not run on
I use login shell in my Linux environments since ever, and I don't see anything dangerous in Also, the login environment would be only applied to the command which we are executing, it would not affect other running shells for example. That said, do you have any reference for what you said? Or an example? |
@chrmarti I'm actually a +1 on doing this by default. There's actually quite a bit of code in vscode-dev-containers that is working around this particular scenario. The big problem is scripts that must be sourced to get the path to work correctly. SDKMAN, nvm, rvm all are like this. I'm adding things the containers install to the PATH when possible, but this only covers a limited set. In the SDKMAN case there are a number of tools it can install. Worse yet, rvm throws up warnings if things are in the path and the targets are not easily predictable (e.g. the gem path). To your point, we could probably add SHELL to work around it, but I could see someone trying to create their own dev container hitting this and being confused. |
The issue with One option is to probe the environment with an interactive non-login shell ( |
@Chuxel Are there any workarounds that cannot be replaced with the recently added |
@chrmarti I don't believe so. I need to do a more complete test. This last release there was enough change already that I opted to keep things as they were. |
@Chuxel If it turns out that you have to use I expect |
The problem of using "interactiveShell" is (as the name says) that the shell is interactive, which means that at any point, bash or zsh can prompt the user for something. Not ideal in this case. @chrmarti merging the two PATHs seems to be an interesting approach. The order would need to be respected as it is today: Docker ENVs comes before the /etc/profile ones. |
@felipecrs Typically CLIs are smart enough to determine when they should prompt even in an interactive shell due to piping. As @chrmarti mentions, |
@Chuxel 'typically' is not a rule, right? The rule is: interactive terminals may ask the user for input. But it will probably work, as you suggest. And the By the way, you're talking about In the end, I [again] suggest using the standard way to customize the system's environment on Linux: adding as many |
I also think that the all sorts of crazy things are not so crazy: $ docker run --rm alpine sh -c 'cat /etc/profile; cat /etc/profile.d/*.sh'
cat: can't open '/etc/profile.d/*.sh': No such file or directory
export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
export PAGER=less
export PS1='\h:\w\$ '
umask 022
for script in /etc/profile.d/*.sh ; do
if [ -r $script ] ; then
. $script
fi
done $ docker run --rm debian sh -c 'cat /etc/profile; cat /etc/profile.d/*.sh'
# /etc/profile: system-wide .profile file for the Bourne shell (sh(1))
# and Bourne compatible shells (bash(1), ksh(1), ash(1), ...).
if [ "`id -u`" -eq 0 ]; then
PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
else
PATH="/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games"
fi
export PATH
if [ "${PS1-}" ]; then
if [ "${BASH-}" ] && [ "$BASH" != "/bin/sh" ]; then
# The file bash.bashrc already sets the default PS1.
# PS1='\h:\w\$ '
if [ -f /etc/bash.bashrc ]; then
. /etc/bash.bashrc
fi
else
if [ "`id -u`" -eq 0 ]; then
PS1='# '
else
PS1='$ '
fi
fi
fi
if [ -d /etc/profile.d ]; then
for i in /etc/profile.d/*.sh; do
if [ -r $i ]; then
. $i
fi
done
unset i
fi
cat: '/etc/profile.d/*.sh': No such file or directory $ docker run --rm ubuntu sh -c 'cat /etc/profile; cat /etc/profile.d/*.sh'
# /etc/profile: system-wide .profile file for the Bourne shell (sh(1))
# and Bourne compatible shells (bash(1), ksh(1), ash(1), ...).
if [ "${PS1-}" ]; then
if [ "${BASH-}" ] && [ "$BASH" != "/bin/sh" ]; then
# The file bash.bashrc already sets the default PS1.
# PS1='\h:\w\$ '
if [ -f /etc/bash.bashrc ]; then
. /etc/bash.bashrc
fi
else
if [ "`id -u`" -eq 0 ]; then
PS1='# '
else
PS1='$ '
fi
fi
fi
if [ -d /etc/profile.d ]; then
for i in /etc/profile.d/*.sh; do
if [ -r $i ]; then
. $i
fi
done
unset i
fi
# Make sure the locale variables are set to valid values.
eval $(/usr/bin/locale-check C.UTF-8) It seems that the only problem is the |
@felipecrs Yep! That's why the property exists - so you can adapt to your situation. |
@Chuxel Yeah, this is true. But the point of this issue is to come up with a better default approach, not only for my situation. The concerns raised about relying on the And as I said, relying on So I think I have a better proposal: A new option in {
"sourceEtcDevcontainer": true // defaults to true
} This option makes the Remote - Containers extension always execute commands in the container using the following syntax: $ docker exec ubuntu sh -c "test -f /etc/devcontainer && . /etc/devcontainer; echo 'hello world'" Unless the user sets it to This will not interfere in The This would enable us to have a clean and unique environment configuration file that would work across all the users ( @Chuxel @chrmarti could you please review this new proposal? |
Note that the 'userEnvProbe' only starts a corresponding shell and uses We would probably use the same approach ( |
* Add back hideHover and use on tree context menu show Fixes microsoft#106268 * Update distro * 💄 * explorer: Fix TrustedTypes violation microsoft#106285 * produce deb, rpm packages * Add loginShell (microsoft/vscode-remote-release#3593) * chore - tweak onDidAddNotebookDocument, onDidRemoveNotebookDocument event, use ResourceMap and fix confusion between models and editors * notebook update * pinned tabs - update setting enum name * Use innerText over innerHTML, microsoft#106285 * rename to IHostColorSchemeService * API proposal for tree item icon color Part of microsoft#103120 * chore - when target might be undefined use `target?.dispose()` over `dispose(target)` * deprecate onDidChangeCells * Reenable notebook smoke test microsoft#105330 * deprecate onDidChangeContent * Add numeric values support for terminal.integrated.fontWeight * unified onDidChangeContent * [email protected] Fixes microsoft#105957 * 💄 * debt - remove _unInitializedDocuments * remove `NotebookDocument#displayOrder` , fixes microsoft#106305 * no uninitialized documents. * chore - update references viewlet * debug: make serverReadyAction play nicely with js-debug Fixes microsoft#86035 Fixes microsoft/vscode-js-debug#440 * fix rpm * high contrast switching in browser * Fix occasional bad custom selectbox layout Fix microsoft#106302 * review comments * Bump vscode-ripgrep for ARM microsoft#6442 * Revert more specific class names for editor tokens Reverts microsoft#103485 Fixes microsoft#106261 I believe that microsoft#103485 broke cases where the markdown renderer creates tokenized strings that are used outside of an editor's dom node (such as in hovers or in webviews) The safest fix for now is to revert it. We can revist this and make the markdown renderer handle the tokenized output better next iteration * remove emit selections. * merge conflict resolve. * fix integration tests. * Disable errors in non-semantic supported files Fixes microsoft#106299 Fixes microsoft#106314 Also enables js/ts features on the right side of PRs and in search results * proper fix for microsoft#105202 (microsoft#106293) * Only enable 'open with' on files Fixes microsoft#106291 * Add WebviewView.description Allow configuring the description for webview views. This is rendered next to the title in a less prominently in the title * Remove manual strikethroughs for deprecated properties in vscode.d.ts Now that TS has support for `@deprecated`, these manual strike throughs should no longer be required. * Add show method to webview view Fixes microsoft#106085 * Skip failing test * fix microsoft#106283 * enable test * fix microsoft#106283 * pinnedTabSizing.standard => pinnedTabSizing.normal * install dot net core sdk * update distro * tabs - align icon and text vertically centered in tab * update distro * distro * fix microsoft#106308 * Update gitignore decorations when .git/info/exclude file is edited (microsoft#106270) * detect local `exclude` file edits * use `uri.path` to detect exclude file edits `uri.path` uses forward slash as a path separator indepentent of the host system, which makes it easier to use with regex * updated searches * editor - rename context keys variables * fix microsoft#105999 * pinned tabs - prevent to close pinned tabs via Cmd+W (microsoft#100738) * Reduce usage of `.innerHTML` (microsoft#106285) * fix uninstalling extension * remove unused import * add `replaceNotebookMetadata` (should become `replaceMetadata`) to NotebookEditorEdit, microsoft#105283 * add `replaceNotebookMetadata`, microsoft#105283 * use textContent instead of innerHTML (for microsoft#106285) * chore - move appyWorkspaceEdit from extHostTextEditors to (new) extHostBulkEdits and mainThreadBulkEdits * chore - extract extHostNotebookDocument for the NotebookDocument and NotebookCell types and friends * chore - extract ExtHostNotebookEditor into its own file * chore - remove ExtHostNotebookEditor#uri and use document.uri instead * chore - 💄 member order: property, ctor, method * publish arm deb and rpm * trusted types related to microsoft#106285 * use async await * distro * update trusted types search * trusted types - use textContent for style elements, fyi @rebornix * fix arch * Fix compile after merge * Use instantiation service to create TerminalLinks * Consolidate colon trim logic * Avoid slice when checking colon * Check length again I prefer chatAt over slice as it's more obvious what's happening * Move comment into helper function * Update extensions/git/src/commands.ts * Update extensions/git/src/commands.ts * Update extensions/git/src/commands.ts * Save prompt is shown while saving from settings split json editor (fix microsoft#106330) * Only allow configurePlugin against main TS Server Fixes microsoft#106346 Looks like the TS Server doesn't support this in partial mode at the moment * Adding more explicit typings for promises This gets us ready for TS 4.1 * Don't use async on abstract functions * chore - use workingCopyService.isDirty instead notebook.isDirty * Update Codicons - Update 'pinned' - Add 'export' - Compress 'merge' microsoft/vscode-codicons@5bcb1a0 * Add explicit undefined parameters / types These errors come from the new stricter types for Promises in TS 4.1 * debt - IMainNotebookController#removeNotebookDocument * debt - invoke resolve notebook when opening a notebook in an editor, not when loading a notebook from source * do not need isUntitled. * 💂 polish nb tests. * remove selections from nb text model. * replace changeCellLanguage to applyEdit * fix microsoft#105916. expand metadata section if modified. * move dirty state to NotebookEditorModel. * chore, simply notebook text model event emitter * refs microsoft#106285 * Add subscribers action * Fix terminal ts 4.1 compile issues Part of microsoft#106358 * Fix ts 4.1 issues in terminal api tests * Update Codicons: add 'graph-left' microsoft/vscode-codicons@dd1edb2 * initialize notebook text model data only through ctor. * 💄 * Mark property readonly * Enable webview commands for webview views Fixes microsoft#105670 Previously our webview commands assumed that webviews were always going to be in an editor. This is no longer true with webview views. To fix this, I've added an `activeWebview` to the `IWebviewService`. This tracks the currently focused webview. * microsoft#106358 * debug: bump js-debug-companion * re microsoft#105735. * re microsoft#105735. no more udpateMetadata api. * Fix microsoft#106303 * Use destructuring to make code more clear * Add isWritableFileSystem api Fixes microsoft#91697 This new API checks if a given scheme allows writing * Revert "Fix microsoft#106303" This reverts commit 8e5eed1. this is causing a layer check issue * Cache webview view title across reloads Fixes microsoft#105867 * fix some TS 4.1 errors (microsoft#106358) * fix some TS 4.1 errors (microsoft#106358) * fix TS 4.1 compile errors, microsoft#106358 * pinned tabs - flip default to "shrink" * fix ts errors related to microsoft#106358 * pinned tabs - closing pinned tab should open next non-pinned * pinned tabs - add a tab.lastPinnedBorder color * Adds commands for --no-verify commit variants (microsoft#106335) * add `{allow,confirm}NoVerifyCommit` options * adds commit comands with no verify * handles no verify command variants * handle no verify commit option * only display no verify variants when option is set * trusted types related to microsoft#106395 * more TS 4.1 fixes (microsoft#106358) * update trusted types search * Fix TS 4.1 errors for tasks and remote explorer Part of microsoft#106358 * Adress microsoft#106358: Fix TS 4.1 errors in codebase * debt - simplify metadata edit because we now have CellEditType.DocumentMetadata * Fix Trusted Types violations (round #2) microsoft#106395 * debug: return result of a msg to debug adapter can be undefined * add ExtHostFileSystemInfo which knows what schemes are reserved and which are used, microsoft#91697 * fixes microsoft#106334 * web - fix bad credentials lookup * Correct path to code-workspace.xml Fixes microsoft#106384 * Multi git executable paths (microsoft#85954) * Multi git executable path * update `git.path` setting description * 💄 Co-authored-by: João Moreno <[email protected]> * Correct linux code-workspace path * fixes microsoft#104047 * Add defaultUriScheme to path service (microsoft#106400) Fixes microsoft/vscode-internalbacklog#1179 * 💄 * Fix microsoft#106303 * Avoid innerHTML (microsoft#106395) * Avoid innerHTML (microsoft#106395) * debt - REMOTE_HOST_SCHEME => Schemas.vscodeRemote * fixes microsoft#106355 * pathService - defaultUriScheme() => defaultUriScheme * Adjust active terminal tab when active tab moves (microsoft#106413) Fixes microsoft#106300 * debt - adopt defaultUriScheme also for userHome * debt - adopt defaultUriScheme over hardcoded vscode-remote in toLocalResource * some integration tests for notebook editing, microsoft#105283 * refs microsoft#106358 * Bump yargs-parser in /extensions/markdown-language-features (microsoft#106373) Bumps [yargs-parser](https://github.com/yargs/yargs-parser) from 13.1.1 to 13.1.2. - [Release notes](https://github.com/yargs/yargs-parser/releases) - [Changelog](https://github.com/yargs/yargs-parser/blob/master/docs/CHANGELOG-full.md) - [Commits](https://github.com/yargs/yargs-parser/commits) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump yargs-parser in /extensions/css-language-features/server Bumps [yargs-parser](https://github.com/yargs/yargs-parser) from 13.1.1 to 13.1.2. - [Release notes](https://github.com/yargs/yargs-parser/releases) - [Changelog](https://github.com/yargs/yargs-parser/blob/master/docs/CHANGELOG-full.md) - [Commits](https://github.com/yargs/yargs-parser/commits) Signed-off-by: dependabot[bot] <[email protected]> * Remove arrays.findIndex For microsoft#103454 This should be a direct map to the `.findIndex` mathod * Use textContnet for style element For microsoft#106395 * use textcontent in menu css refs microsoft#106395 * Fix one innerHTML usage microsoft#106395 * Use `@example` tags in vscode.d.ts (microsoft#106352) `@example` is the standard way to document code examples. This also gets us syntax highlighting of code examples in hovers * - reload only local user configuration after initi - add perf mark up and logs * re microsoft#105735. batch apply edits. * notebook text model initialization does not increment version * private outputs slice and unknown change. * applyEdit supports begin/end selections. * replace insertCell with applyEdit. * do not copy execution related metadata * 💄 * fix build. * Document view.type contribution Fixes microsoft#105821 * Improve views contribution point - add required properties - add default snippet - use `markdownDescription` for markdown string * Replace our arrays.find with stdlib find For microsoft#103454 * Pin blob storage dep see Azure/azure-sdk-for-js#11187 * unit tests for batched edits. * remove spliceNotebook api from textmodel. * update exploration branch * Fix some trusted type violations, microsoft#106395 * fix fonts in monaco menu * update distro * some jsdoc for microsoft#54938 * try to fix build (linux) * electron - set spellcheck: false again for windows * update search file, microsoft#106395 * 🆙 web playground * Trusted types - don't use innerHTML for rapid render, microsoft#106395 * Remove Schemas.vscodeRemote from simple file dialog * debt - remove some any casts from window * update distro * fix linux build * argh * proxy authentication does not work on 1.49 (microsoft#106434) * do not use hasClass and first microsoft#103454 * debug and explroer: do not use dom.addClass, dom.toggleClass * do not use deprecated dom helper methods microsoft#103454 * adopt new amd loader with support for TrustedScriptURL, add typings for TrustedTypesFactory et al, microsoft#106396 * explorer: Should maintain row focus after deleting a file fixes microsoft#71315 * Update Codicons: add 'magnet' icon microsoft/vscode-codicons@4c61155 * Remove unused 'SettingSearch' issue type * notebook document data loss. * cell language should not be freezed. * Add preferred_username to the list of msft token claims (microsoft#106511) * debug: update js-debug * fix microsoft#106430. * hide outputs if it is transient. * Add optional typing for webview state in WebviewPanelSerializer This makes it easier for extensions to correctly type their state if they wish * Add comment to WebviewViewResolveContext * use optional chaining * Use `Set` instead of array Sets should offer faster checking to see if a property has been seen * Create webview.web.contribution Fixes microsoft#106516 Creates an explicit contribution file for web. This makes sure we only don't register the `IWebviewService` twice. Not 100% sure this fixes the issue since I couldn't repo the original bug with our oss builds * Revert "API proposal for tree item icon color" This reverts commit 52e557f. * Skip formatting when during format-on-save, the configured formatter cannot be found (continue to show silent notification), microsoft#106376 * don't use renderCodicons any more, microsoft#105799 * remove old renderCodicons-function, rename renderCodiconsAsElement to renderCodicons * NotebookEditorEdit-api changes, microsoft#105283 * WorkspaceEdit-api changes, microsoft#105283 * adopt notebook integration tests, microsoft#105283 * add NotebookCell#index, microsoft#106637 * fix delay issue for provideCodeLenses, microsoft#106267 * rename RunOnceScheduler#timeout to delay * use debian stretch images (microsoft#106656) * remove deprecated function calls related to microsoft#103454 * workaround, maybe fix for microsoft#106657 * update search files * debt - make class list utils functions so that @deprecated works for them * fixes microsoft#106406 * notebook - when creating a notebook, check that no notebook with another viewtype exists * fix bad classList usage * add regression test for microsoft#106657 * fixes microsoft#86180 * fixes microsoft#100172 Co-authored-by: Alex Ross <[email protected]> Co-authored-by: Daniel Imms <[email protected]> Co-authored-by: João Moreno <[email protected]> Co-authored-by: isidor <[email protected]> Co-authored-by: Christof Marti <[email protected]> Co-authored-by: Johannes Rieken <[email protected]> Co-authored-by: Benjamin Pasero <[email protected]> Co-authored-by: Martin Aeschlimann <[email protected]> Co-authored-by: rebornix <[email protected]> Co-authored-by: Rob Lourens <[email protected]> Co-authored-by: IllusionMH <[email protected]> Co-authored-by: Daniel Imms <[email protected]> Co-authored-by: Connor Peet <[email protected]> Co-authored-by: Matt Bierner <[email protected]> Co-authored-by: Jean Pierre <[email protected]> Co-authored-by: Peng Lyu <[email protected]> Co-authored-by: Sandeep Somavarapu <[email protected]> Co-authored-by: Vyacheslav Pukhanov <[email protected]> Co-authored-by: Alex Dima <[email protected]> Co-authored-by: João Moreno <[email protected]> Co-authored-by: Miguel Solorio <[email protected]> Co-authored-by: SteVen Batten <[email protected]> Co-authored-by: Jackson Kearl <[email protected]> Co-authored-by: Rachel Macfarlane <[email protected]> Co-authored-by: Dirk Baeumer <[email protected]> Co-authored-by: WhizSid <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: deepak1556 <[email protected]> Co-authored-by: Oleg Demchenko <[email protected]>
Looks like a duplicate of #3585 |
True, pretty much the same. I'll update the PR title and description to match #3593 (comment), so they'll be different. |
I rewrote this Issue. |
Another thing, ENV BASH_ENV="/etc/devcontainer"
SHELL ["/bin/bash", "-c"]
RUN echo "/etc/devcontainer will be sourced" As it's an RUN printf '%s\n' '' 'test -f /etc/devcontainer && . /etc/devcontainer' | sudo tee /etc/bash.bashrc /etc/zsh/zshrc |
One problem with userEnvProbe is that it's not available for containers you attach to using attach to existing container option #6226 , /etc/devcontainer config file would potentially solve that. |
@QAston You can open the config file associated with the 'attach' container using |
@chrmarti thanks, I didn't notice it because it's not documented in the reference: https://code.visualstudio.com/docs/remote/devcontainerjson-reference#_attached-container-configuration-reference |
@QAston Thanks, opened a PR for the documentation. (microsoft/vscode-docs#5119) |
A new option in
devcontainer.json
:This option makes the Remote - Containers extension always execute commands in the container using the following syntax:
$ docker exec ubuntu sh -c "test -f /etc/devcontainer && . /etc/devcontainer; echo 'hello world'"
Unless the user sets it to
false
.This will not interfere in
userEnvProbe
behavior.The
/etc/devcontainer
file must be a shell script compatible with Bourne Shell/bin/sh
.This would enable us to have a clean and unique environment configuration file that would work across all the users (
root
and non-root), and all the shells, without the caveats of/etc/profile
orbash.bashrc
.So global environment variables can be set using scripts and not only through the
Dockerfile
'sENV
statement, or thedevcontainer.json
'scontainerEnv
, thus making the installation scripts easier to use since they would not require the user to manually inject some statements in theDockerfile
.This would mainly benefit the scripts from
script-library
onvscode-dev-containers
repository, which are used for doing common customizations on the containers. As explained here (bullet number 2).The text was updated successfully, but these errors were encountered: