Skip to content
This repository was archived by the owner on Nov 30, 2023. It is now read-only.

Conversation

@bhack
Copy link
Contributor

@bhack bhack commented Mar 16, 2020

No description provided.

@bhack
Copy link
Contributor Author

bhack commented Mar 16, 2020

Also can we remove some other dependencies seems that we are installing openjdk etc..?

@Chuxel
Copy link
Member

Chuxel commented Mar 16, 2020

@bhack These are common scripts that layer in the most common utilities and things VS Code extensions may need. There's no requirement that a definition use these scripts but, when included, most extensions should have their needed dependencies covered. It also installs a few apt-get dependencies that make typical CLI install instructions work for those extending the Dockerfiles.

The thing that drives the largest dependency graph is actually git and openssh-client which are required to manage source control and use SSH keys respectively.

Java / openjdk is not automatically installed to my knowledge. Python is present since this is an OS dependency for many tools (and again is not explicitly installed). If I recall correctly, git is one of them. If you want to get a sense of what is present after just this script is run, you can quickly use docker run with the base image (debian-10) as follows:

docker run -it --rm mcr.microsoft.com/vscode/devcontainers/base:debian-10

@Chuxel Chuxel added the info-needed Issue requires more information from poster label Mar 16, 2020
@bhack
Copy link
Contributor Author

bhack commented Mar 16, 2020

Ok, thanks probably was another layer to introduce Jdk in my test.
But this PR is still valid cause fix hardcoded libssl.

@Chuxel
Copy link
Member

Chuxel commented Mar 16, 2020

This installs 1.0.x (if available in the distro) and 1.1. Is there a problem that installing 1.1 is causing? What advantage does removing it provide?

@bhack
Copy link
Contributor Author

bhack commented Mar 16, 2020

@Chuxel
Copy link
Member

Chuxel commented Mar 16, 2020

Debian 10 dropped libssl1.0.x. Debian 9 has libssl1.0.2. Ubuntu 18.04 and 16.04 have libssl1.0.0.

All three include libssl1.1

libssl1.1 is not backwards compatible to libssl1.0, so both need to be on disk in some cases. That's why they're peer packages.

@bhack
Copy link
Contributor Author

bhack commented Mar 16, 2020

@bhack
Copy link
Contributor Author

bhack commented Mar 16, 2020

I meant have you seen my previous libssl packages list in 16.04?

@bhack
Copy link
Contributor Author

bhack commented Mar 16, 2020

libssl1.1 was introduced in 18.04 bionic see https://packages.ubuntu.com/search?suite=bionic&searchon=names&keywords=libssl

@Chuxel
Copy link
Member

Chuxel commented Mar 16, 2020

Ah, that's a separate issue. Nothing that uses the script right now uses Ubuntu 16.04. That said, we could make the installation of 1.1 conditional like libssl1.0.x to resolve that.

@bhack
Copy link
Contributor Author

bhack commented Mar 16, 2020

Do you like --ignore-missing or it is a too bad hack?

@bhack
Copy link
Contributor Author

bhack commented Mar 16, 2020

Can we use libssl1.[0-1].[0-9] and remove the bash stuff?

@Chuxel
Copy link
Member

Chuxel commented Mar 16, 2020

@bhack There have been problems we've seen in the past with not being specific here particularly if someone adds a backport to install something. This code was introduced to mitigate that problem. The check is simple and has avoided any issues related to it since.

This should do it.

if [[ ! -z $(apt-cache --names-only search ^libssl1.1$) ]]; then
        apt-get -y install  --no-install-recommends libssl1.1
 fi

@bhack
Copy link
Contributor Author

bhack commented Mar 16, 2020

Do you like now?

@Chuxel
Copy link
Member

Chuxel commented Mar 16, 2020

LGTM!

@Chuxel Chuxel merged commit b53289b into microsoft:master Mar 16, 2020
@bhack bhack deleted the fix_libssl branch March 16, 2020 18:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

info-needed Issue requires more information from poster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants