New multi docker #3902
Closed
New multi docker #3902
Conversation
This file contains 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
heliocastro
force-pushed
the
new_multi_docker
branch
8 times, most recently
from
c1a53b3
to
cfce639
Compare
heliocastro
force-pushed
the
new_multi_docker
branch
4 times, most recently
from
3616688
to
e34655d
Compare
sschuberth
added
docker
|
@heliocastro thanks for contributing! The Python support in ORT as-is is indeed limited to Python 3.6 and supporting other Python versions makes a lot of sense. Deciding for the way to go would be a pre-requisite for reviewing this PR. I'm sorry to tell that we don't have the time currently to do that in the near future. |
heliocastro
force-pushed
the
new_multi_docker
branch
4 times, most recently
from
1030b55
to
bcc8d05
Compare
tsteenbe
reviewed
Jul 16, 2021
tsteenbe
requested changes
Jul 16, 2021
|
@heliocastro The "Update to use pyenv" commit is not signed off that why DCO check fails - could you also add a commit message to this commit? |
tsteenbe
reviewed
Jul 16, 2021
|
@heliocastro How difficult would it be to also build a massive source tarbal for this Docker image? |
|
I'm glad with the rework, mostly the update of ScanCode Toolkit and the ability to change both Python and Node versions |
|
Seems really cool, thanks @heliocastro . I've successfully compiled it with docker, but when running the pip package manager I always end up with the following: What am I doing wrong? |
|
It seems to work (although I still get errors due to private dependencies), when I first run Initially I was having trouble with virtualenv due to a version restriction: I resolved that version restriction by updating the versions in NOTE: there is a warning in the code that pip 20.3 comes with a new dependency resolver that is untested with ORT. |
heliocastro
force-pushed
the
new_multi_docker
branch
from
45b993a
to
9cfba50
Compare
maxhbr
reviewed
Sep 8, 2021
|
|
||
| add_local_path "${PYENV_ROOT}/bin" | ||
|
|
||
| eval "$(pyenv init --path)"; |
There was a problem hiding this comment.
Suggested change
| eval "$(pyenv init --path)"; | |
| eval "$(pyenv init --path)" |
|
|
||
| eval "$(pyenv init --path)"; | ||
| #shellcheck disable=1091 | ||
| . "$(pyenv root)"/completions/pyenv.bash; |
There was a problem hiding this comment.
Suggested change
| . "$(pyenv root)"/completions/pyenv.bash; | |
| . "$(pyenv root)"/completions/pyenv.bash |
Dockerfile
Outdated
| && npm install --global npm@$NPM_VERSION bower@$BOWER_VERSION yarn@$YARN_VERSION | ||
|
|
||
| #------------------------------------------------------------------------ | ||
| # ORT |
Dockerfile
Outdated
| && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ | ||
| build-essential \ | ||
| dirmngr \ | ||
| dpkg-dev \ |
There was a problem hiding this comment.
indentation is mixed up here with tabs and spaces
Dockerfile
Outdated
|
|
||
| #------------------------------------------------------------------------ | ||
| # Scancode from official releases | ||
| ARG SCANCODE_VERSION=21.8.4 |
There was a problem hiding this comment.
Why is scancode added after ORT source? Wouldn't it be better for caching to reverse that?
maxhbr
previously approved these changes
Sep 8, 2021
|
@heliocastro : I suggested some changes in heliocastro#16 |
|
I build a new Docker image yesterday with the default settings and tried to run it for a Python project. It seems ORT is not able to access virtualenv: Trying to debug it shows virtualenv is not readily available: Edit: Backtracking the So as previously the |
heliocastro
force-pushed
the
new_multi_docker
branch
from
3075760
to
d25349b
Compare
@nicorikken This is already solved in latest commits. virtualenv is installed in the current python, but temember that if you are using interactive environment, you will need run the wrappers to initialize pyenv |
|
FYI: new scancode release: https://github.com/nexB/scancode-toolkit/releases/tag/v30.1.0 |
heliocastro
force-pushed
the
new_multi_docker
branch
from
d25349b
to
46c942e
Compare
|
I think dart support might be broken in this PR. When scanning antlr4 it fails with the following error: Might also be broken on master, I have not tested that. |
maxhbr
suggested changes
Oct 16, 2021
|
|
||
| #------------------------------------------------------------------------ | ||
| # Scancode from official releases | ||
| ARG SCANCODE_VERSION=30.1.0 |
There was a problem hiding this comment.
I think you also need to change the scancodeVersion in ./gradle.properties.
|
@heliocastro Instead of nvm what do you thing of using n which seems to have a simpler API for example see https://blog.insiderattack.net/working-with-multiple-nodejs-versions-85c8eef7a600 |
* Switchable build arg versions * Scancode build from releases, not from python npip * Python uses pyenv. Default python is 3.8.11 * Nodejs is using nvenv. Default nodejs is the latest LTS * Ruby uses rbenv. Bundler and Cocoapods now come by default * Wrapper script to use bashrc on direct ort usage * Use bash and pipefail on all operations as default. Exception for Android sdkmanager that behaves differently on pipefail * Silence gpg dearmor output on tee * Using a single downloader, curl * Go updated to last stable release Signed-off-by: Helio Chissini de Castro <helio@kde.org>
Signed-off-by: Maximilian Huber <gh@maxhbr.de>
Signed-off-by: Helio Chissini de Castro <helio@kde.org>
Signed-off-by: Helio Chissini de Castro <helio@kde.org>
For some not yet identified, defauly Ubuntu openjdk 11 fail to build current instance of Ort upstream code. AdoptOpenJDk fix the issue as a better solution. Signed-off-by: Helio Chissini de Castro <helio@kde.org>
heliocastro
force-pushed
the
new_multi_docker
branch
from
46c942e
to
c469070
Compare
|
Superseded by #4746. |
|
Would be nice if you just push the docker-image to Docker Hub |
Please refer to #2441 for that matter instead of commenting on closed PRs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Dockerfile: Multidocker based on Ubuntu 20.04
Android sdkmanager that behaves differently on pipefail