Skip to content

refactor: Use environments to install Conan #10127

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

refactor: Use environments to install Conan #10127

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bfb8a2c
refactor(docker): Use environments to install Conan
heliocastro Apr 4, 2025
321ae2a
fix(conan): Adapt the Conan package manager to use the virtual enviro…
nnobelis Apr 10, 2025
6f90359
fix(conan)!: Use a Conan profile named `ort-default` instead of the d…
nnobelis Apr 10, 2025
76b591b
refactor(conan): Remove dummy profile settings
heliocastro Apr 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ ARG PYTHON_VERSION
ARG PYENV_GIT_TAG

ENV PYENV_ROOT=/opt/python
ENV PATH=$PATH:$PYENV_ROOT/shims:$PYENV_ROOT/bin:$PYENV_ROOT/conan2/bin
ENV PATH=$PATH:$PYENV_ROOT/shims:$PYENV_ROOT/bin
Copy link
Member

@sschuberth sschuberth Jun 6, 2025

Choose a reason for hiding this comment

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

IMO the commit message needs to be reworked. To me, using dashes for a list / an enumeration is always an alarm sign that probably too much work has been crammed into a single commit.

I propose something like:

Migrate the previous approach to install binaries named `conan` and
`conan2` to properly separated virtualenvs for Python. This is done via the
pyenv-virtualenv plugin [1] for pyenv [2].

[1]: https://github.com/pyenv/pyenv-virtualenv
[2]: https://github.com/pyenv/pyenv

RUN curl -kSs https://pyenv.run | bash \
&& pyenv install -v $PYTHON_VERSION \
&& pyenv global $PYTHON_VERSION
Expand Down Expand Up @@ -170,18 +170,26 @@ RUN pip install --no-cache-dir -U \
wheel \
&& pip install --no-cache-dir -U \
Mercurial \
conan=="$CONAN_VERSION" \
pipenv=="$PYTHON_PIPENV_VERSION" \
poetry=="$PYTHON_POETRY_VERSION" \
poetry-plugin-export=="$PYTHON_POETRY_PLUGIN_EXPORT_VERSION" \
python-inspector=="$PYTHON_INSPECTOR_VERSION" \
setuptools=="$PYTHON_SETUPTOOLS_VERSION"
RUN mkdir /tmp/conan2 && cd /tmp/conan2 \
&& wget https://github.com/conan-io/conan/releases/download/$CONAN2_VERSION/conan-$CONAN2_VERSION-linux-x86_64.tgz \
&& tar -xvf conan-$CONAN2_VERSION-linux-x86_64.tgz\
# Rename the Conan 2 executable to "conan2" to be able to call both Conan version from the package manager.
&& mkdir $PYENV_ROOT/conan2 && mv /tmp/conan2/bin $PYENV_ROOT/conan2/ \
&& mv $PYENV_ROOT/conan2/bin/conan $PYENV_ROOT/conan2/bin/conan2

# Create conan environments
COPY scripts/setup_conan.sh ${PYENV_ROOT}/bin/conan
RUN eval "$(pyenv init - bash)" \
&& eval "$(pyenv virtualenv-init -)" \
&& pyenv virtualenv conan \
&& pyenv activate conan \
&& pip install conan==${CONAN_VERSION} \
&& pyenv deactivate \
&& pyenv virtualenv conan2 \
&& pyenv activate conan2 \
&& pip install conan==${CONAN2_VERSION} \
&& pyenv deactivate \
&& sudo chmod +x ${PYENV_ROOT}/bin/conan
Comment on lines +181 to +191

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 5: pipCommand not pinned by hash
Click Remediation section below to solve this issue
Comment on lines +181 to +191

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 5: pipCommand not pinned by hash
Click Remediation section below to solve this issue


FROM scratch AS python
COPY --from=pythonbuild /opt/python /opt/python
Expand Down Expand Up @@ -484,7 +492,7 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \

# Python
ENV PYENV_ROOT=/opt/python
ENV PATH=$PATH:$PYENV_ROOT/shims:$PYENV_ROOT/bin:$PYENV_ROOT/conan2/bin
ENV PATH=$PATH:$PYENV_ROOT/shims:$PYENV_ROOT/bin:$PYENV_ROOT/plugins/pyenv-virtualenv/shims
COPY --from=python --chown=$USER:$USER $PYENV_ROOT $PYENV_ROOT

# NodeJS
Expand Down
2 changes: 1 addition & 1 deletion plugins/package-managers/conan/src/main/kotlin/Conan.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ internal class ConanCommand(private val useConan2: Boolean = false) : CommandLin
override fun getVersionRequirement(): RangesList = RangesListFactory.create(">=1.44.0 <3.0")

override fun run(vararg args: CharSequence, workingDir: File?, environment: Map<String, String>) =
super.run(args = args, workingDir, environment + ("CONAN_NON_INTERACTIVE" to "1"))
super.run(args = args, workingDir, environment + mapOf("CONAN_NON_INTERACTIVE" to "1", "CONAN_SERIES" to "1"))
Copy link
Member

@sschuberth sschuberth Jun 6, 2025

Choose a reason for hiding this comment

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

This still refer to CONAN_SERIES instead of CONAN_MAJOR_VERSION. But is it even correct to hard-code this here? Shouldn't it be dropped to make run work for both major versions?

}

data class ConanConfig(
Expand Down
45 changes: 45 additions & 0 deletions scripts/setup_conan.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#!/bin/bash
#
# Copyright (C) 2025 The ORT Project Authors (see <https://github.com/oss-review-toolkit/ort/blob/main/NOTICE>)
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# SPDX-License-Identifier: Apache-2.0
# License-Filename: LICENSE
#

conan_option=${CONAN_MAJOR_VERSION:-2}
Copy link
Member

@sschuberth sschuberth Jun 6, 2025

Choose a reason for hiding this comment

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

How about making this

CONAN_VENV="conan${CONAN_MAJOR_VERSION:-2}"

and then below simply call

pyenv activate $CONAN_VENV

once?

However, that would require to rename the venv from "conan" to "conan1", but maybe it's not a bad ideal to be explicit anyway.


# Since this script is installed with the name "conan", there is a risk of infinite recursion if pyenv is not available
# on the PATH, which can occur when setting up a development environment. To prevent this, check for recursive calls.
if [[ "$CONAN_RECURSIVE_CALL" -eq 1 ]]; then
Copy link
Member

@sschuberth sschuberth Jun 6, 2025

Choose a reason for hiding this comment

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

A simpler check could be to compare ${BASH_SOURCE[1]} with this script's path, i.e. ${BASH_SOURCE[0]}.

echo "Recursive call detected. Exiting."
exit 1
fi

# Setup pyenv
eval "$(pyenv init - --no-rehash bash)"
eval "$(pyenv virtualenv-init -)"

# Setting up Conan 1.x
if [[ "$conan_option" -eq 1 ]]; then # Setting up Conan 1.x series
pyenv activate conan
# Docker has modern libc
CONAN_RECURSIVE_CALL=1 conan profile update settings.compiler.libcxx=libstdc++11 ort-default
Copy link
Member

@sschuberth sschuberth Jun 6, 2025

Choose a reason for hiding this comment

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

Why does this only need to be done for Conan 1?

elif [[ "$conan_option" -eq 2 ]]; then # Setting up Conan 2.x series
pyenv activate conan2
fi

# Runs conan from activated profile
CONAN_RECURSIVE_CALL=1 conan "$@"