Skip to content
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

Conda pack support #213

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conda pack support #213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cb8b8df
scripts: Extract common functions used in env scripts
PiotrZierhoffer Oct 29, 2019
9c615cd
scripts: Add build-pack.sh script
PiotrZierhoffer Oct 29, 2019
c375d29
scripts: Add enter-pack.sh script
PiotrZierhoffer Oct 30, 2019
cbda6eb
scripts: Prevent conda-pack activation from setting excessive env vars
PiotrZierhoffer Oct 31, 2019
e462a06
scripts: Remove old conda pack before building the new one
PiotrZierhoffer Nov 1, 2019
a8acfac
Add comments to template-environment.yml
mateusz-holenko Oct 31, 2019
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
155 changes: 155 additions & 0 deletions scripts/build-pack.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
#!/bin/bash

### This script is, in principle, similar to download-env.sh
### Notable differences:
### - download-env does not download everything, but depends on system variables
### - download-env installs LiteX submodules. This is not allowed here as
### conda-pack fails when it finds editable pip modules
### - download-env downloads Zephyr SDK if needed. This script does not do it,
### but will when the SDK conda package is created

SETUP_SRC="$(realpath ${BASH_SOURCE[0]})"
SETUP_DIR="$(dirname "${SETUP_SRC}")"
TOP_DIR="$(realpath "${SETUP_DIR}/..")"

source "$SETUP_DIR/setup-helpers.sh"
source "$SETUP_DIR/settings.sh"

verify_system || exit 1

set -e
set -u

# Ensure the build dir
mkdir -p $BUILD_DIR

echo ""
echo "Initializing environment"
echo "------------------------"
# Install and setup conda for downloading packages
export PATH=$CONDA_DIR/bin:$PATH:/sbin
(
install_conda
)

echo ""
echo "Preparing packages"
echo "------------------"

# this file will contain final env description
ENV_FILE=$BUILD_DIR/environment.yml

# this file contains packages which versions are not specified in settings.sh
ENV_TEMPLATE=$SETUP_DIR/template-environment.yml

# This is the name of the new environment from which the pack will be created.
# Note that this name appears in the template file as well. These values should
# match each other
ENV_NAME=lxbe-env

# As conda environment yml format does not seem to allow variables and we want
# to rely on settings.sh to specify package versions, we programatically fill up
# the env file with proper versions.
cat $ENV_TEMPLATE > $ENV_FILE
echo " - conda=$CONDA_VERSION" >> $ENV_FILE
echo " - python=$PYTHON_VERSION" >> $ENV_FILE
echo " - openocd=$OPENOCD_VERSION" >> $ENV_FILE
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to extract the versions from the environment.yml file and remove the settings.sh file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it makes sense, parsing it will be trivial and will allow us to drop this quasi-auto-generation of yml. It's a shame conda does not allow variables substitution. Even python-pip does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mithro So we gave some thought to it and this calls for more discussion.

  • download-env.sh installs packages on per-need basis
  • build-pack.sh installs all the packages at once
  • the versions are the same in both scenarios
  • some packages are downloaded via pip, some are via conda.

Right now cmake is the only pip-installed package that has a version specified. It turns out that Conda has it in an older, but still suitable version available.

This means we could have multiple requirements.txt files in scripts/dependencies. There could be requirements.txt for common stuff, requirements-zephyr.txt, requirements-tinyfpga_b2.txt etc.

This way we could:
a) have a clear distinction over what requires which packages
b) install via pip install -r in both download-env and build-pack.

For Conda we can do it similarly, and have multiple requirement files for different dependencies, and then use them all to create a pack.
There are, allegedly, some problems with using files as source for conda install, but I'm sure it's manageable. The format is also not really specified, but I think that pip files work fine, just as yml files.

Parsing files is something we'd prefer to avoid, for example to be able to easily switch the format/backend in the future.

Summing up, the flow would be:

download-env.sh:
if zephyr then ensure_deps_for(zephyr)
if sth_else then ensure_deps_for(sth_else)

build-pack.sh:
ensure_all_deps()

ensure_deps_for(x):
    // in future: support docker/local installations
    conda install -f conda_deps_for_$x
    pip install -r pip_deps_for_$x

ensure_all_deps():
   for x in deps:
      ensure_deps_for(x)

What's your opinion on that?

Copy link
Member

Choose a reason for hiding this comment

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

I like the idea of having;

  • requirements-<board>.txt
  • requirements-<firmware>.txt
  • requirements-<cpu>.txt

I'm unclear if it should be separate firmware / cpu files or if it should be requirements-<firmware>-<cpu>?

Another option is that we could use dummy conda packages with the requirements as dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe I will be enough to have requirements-<something>.txt, with only one dimension. If more is needed, we can always add it.


# This code is supposed to add entries for all architectures and toolchain
# flavors.
# Some conda packages are not there yet (namely x86_64 and lm32 with musl),
# so we comment these out.
for ARCH in or1k riscv32 lm32 x86_64; do
Copy link
Member

Choose a reason for hiding this comment

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

I don't think x86_64 really makes sense? Or?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not per se, it was actually an emulation of CPU=none. Is there any reasonable usage of this setting at this moment?

Copy link
Member

Choose a reason for hiding this comment

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

Actually CPU=none is a good thing for us to support, so lets make sure it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can it be used in a meaningful way? I mean, how do we define "works"?

Copy link
Member

Choose a reason for hiding this comment

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

For a compiler, test it is able to compile something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean x86 compiler? I don't really understand the way you'd like to use it. CPU=none ./scripts/build-linux.sh could make sense in a way, but it has nothing to do with LiteX or the compilers we usually use, it's just a generic kernel compilation within a conda env. The same applies to Zephyr or micropython.

To effectively include this option in the general flow I need to understand how can it be useful.

if [[ "$ARCH" == "x86_64" ]]; then
echo -n "#" >> $ENV_FILE
fi
echo " - binutils-$ARCH-elf=$BINUTILS_VERSION" >> $ENV_FILE
for FLAVOR in elf-nostdc elf-newlib linux-musl; do
if [[ "$ARCH" == "x86_64" || ( "$ARCH" == "lm32"
Copy link
Member

Choose a reason for hiding this comment

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

A case statement here is probably clearer.

&& "$FLAVOR" == "linux-musl" ) ]]; then
echo -n "#" >> $ENV_FILE
fi
echo " - gcc-$ARCH-$FLAVOR=$GCC_VERSION" >> $ENV_FILE
done
done

echo ""
echo "Preparing dependencies"
echo "----------------------"
cat $ENV_FILE

# This creates an lxbe-env environment.
conda env create -f $ENV_FILE

#required to not rely on user site-packages
export PYTHONNOUSERSITE=1

# We enter the conda env to install things via pip manually.
# This can't be done via the yml file because we invariably use the user's
# site-packages.
# Unfortunately, the activate/deactivate scripts have to have -e/-u disabled.
# This also applies to Renode package installation.
set +u
set +e
source activate $ENV_NAME

# We install Renode separately, no to polute the ENV_FILE with conda-forge repo.
# It is needed for mono.
conda install -y $CONDA_FLAGS -c antmicro -c conda-forge renode

set -u
set -e

# We're using pip install instead of putting it in the yml, because we want to
# ensure isolation.
# conda env create does not seem to take PYTHONNOUSERSITE=1 very seriously.
# We're not installing conda-pack from conda because it requires conda-forge
# source.
pip install conda-pack \
Copy link
Member

Choose a reason for hiding this comment

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

Can you use a requirements.txt file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please see #213 (comment)

tinyfpgab \
progressbar2 \
colorama \
sphinx_rtd_theme \
sphinx \
pyelftools \
west \
pykwalify \
"git+https://github.com/tinyfpga/TinyFPGA-Bootloader#egg=tinyprog&subdirectory=programmer&subdirectory=programmer" \
git+https://github.com/mithro/hexfile.git \
git+https://github.com/timvideos/HDMI2USB-mode-switch.git \
"cmake==$CMAKE_VERSION"


# This has to be installed manually in the environment
MIMASV2CONFIG=$BUILD_DIR/conda/bin/MimasV2Config.py
Copy link
Member

Choose a reason for hiding this comment

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

Let's make it into a Python package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I don't think it would make sense to PR to the original repo, do you want to have it in timvideos GH?

Copy link
Member

@mithro mithro Nov 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It does make sense to PR to the original repo, they will probably accept the pull request.

I have contacts at Numato if you run into trouble.

echo
echo "Installing MimasV2Config.py (mimasv2 flashing tool)"
if [ ! -e $MIMASV2CONFIG ]; then
wget https://raw.githubusercontent.com/numato/samplecode/master/FPGA/MimasV2/tools/configuration/python/MimasV2Config.py -O $MIMASV2CONFIG
chmod a+x $MIMASV2CONFIG
fi
check_exists MimasV2Config.py

echo ""
echo "Working environment"
echo "-------------------"

conda env export

echo ""
echo "Creating a pack"
echo "---------------"

rm -f $BUILD_DIR/$ENV_NAME.tar.gz
conda pack -p $CONDA_DIR/envs/$ENV_NAME -o $BUILD_DIR/$ENV_NAME.tar.gz

# The activate/deactivate conda scripts have to have -e/-u disabled.
set +u
set +e
conda deactivate
set -e
set -u

echo ""
echo "Package created in $BUILD_DIR/$ENV_NAME.tar.gz!"
echo ""
176 changes: 3 additions & 173 deletions scripts/download-env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,51 +24,9 @@ if [ $SOURCED = 1 ]; then
return
fi

source "$SETUP_DIR/setup-helpers.sh"





if [ ! -z "$SETTINGS_FILE" -o ! -z "$XILINX" ]; then
echo "You appear to have sourced the Xilinx ISE settings, these are incompatible with setting up."
echo "Please exit this terminal and run again from a clean shell."
exit 1
fi

# Conda does not support ' ' in the path (it bails early).
if echo "${SETUP_DIR}" | grep -q ' '; then
echo "You appear to have whitespace characters in the path to this script."
echo "Please move this repository to another path that does not contain whitespace."
exit 1
fi

# Conda does not support ':' in the path (it fails to install python).
if echo "${SETUP_DIR}" | grep -q ':'; then
echo "You appear to have ':' characters in the path to this script."
echo "Please move this repository to another path that does not contain this character."
exit 1
fi

# Check ixo-usb-jtag *isn't* installed
if [ -e /lib/udev/rules.d/85-ixo-usb-jtag.rules ]; then
echo "Please uninstall ixo-usb-jtag package from the timvideos PPA, the"
echo "required firmware is included in the HDMI2USB modeswitch tool."
echo
echo "On Debian/Ubuntu run:"
echo " sudo apt-get remove ixo-usb-jtag"
echo
exit 1
fi

#if [ -f /etc/udev/rules.d/99-hdmi2usb-permissions.rules -o -f /lib/udev/rules.d/99-hdmi2usb-permissions.rules -o -f /lib/udev/rules.d/60-hdmi2usb-udev.rules -o ! -z "$HDMI2USB_UDEV_IGNORE" ]; then
# true
#else
# echo "Please install the HDMI2USB udev rules."
# echo "These are installed by scripts/debian-setup.sh"
# echo
# exit 1
#fi

verify_system || exit 1

set -e

Expand Down Expand Up @@ -130,109 +88,6 @@ if [ -z "$PLATFORM" -o ! -e ${TOP_DIR}/targets/$PLATFORM ]; then
exit 1
fi

function check_exists {
TOOL=$1
if which $TOOL >/dev/null; then
echo "$TOOL found at $(which $TOOL)"
return 0
else
echo "$TOOL *NOT* found"
echo "Please try running the $SETUP_DIR/download-env.sh script again."
return 1
fi
}

function check_version {
TOOL=$1
VERSION=$2
if $TOOL --version 2>&1 | grep -q $VERSION > /dev/null; then
echo "$TOOL found at $VERSION"
return 0
else
$TOOL --version
echo "$TOOL (version $VERSION) *NOT* found"
echo "Please try running the $SETUP_DIR/download-env.sh script again."
return 1
fi
}

function check_import {
MODULE=$1
if python3 -c "import $MODULE"; then
echo "$MODULE found"
return 0
else
echo "$MODULE *NOT* found!"
echo "Please try running the $SETUP_DIR/download-env.sh script again."
return 1
fi
}

function check_import_version {
MODULE=$1
EXPECT_VERSION=$2
ACTUAL_VERSION=$(python3 -c "import $MODULE; print($MODULE.__version__)")
if echo "$ACTUAL_VERSION" | grep -q $EXPECT_VERSION > /dev/null; then
echo "$MODULE found at $ACTUAL_VERSION"
return 0
else
echo "$MODULE (version $EXPECT_VERSION) *NOT* found!"
echo "Please try running the $SETUP_DIR/download-env.sh script again."
return 1
fi
}

function fix_conda {
for CONDA_SITE_PACKAGES in $(realpath $CONDA_DIR/lib/python*/site-packages/); do
CONDA_COMMON_PATH="$CONDA_SITE_PACKAGES/conda/common/path.py"
if [ ! -e $CONDA_COMMON_PATH ]; then
continue
fi
if grep -q "def expanduser" $CONDA_COMMON_PATH; then
echo "In $CONDA_SITE_PACKAGES conda/common/path.py is already patched."
continue
fi
START_SUM=$(sha256sum $CONDA_COMMON_PATH | sed -e's/ .*//')
(echo -n "In $CONDA_SITE_PACKAGES " && cd $CONDA_SITE_PACKAGES && patch -p1 || exit 1) <<EOF
diff --git a/conda/common/path.py b/conda/common/path.py
index 0228a3d0b..ffb879a39 100644
--- a/conda/common/path.py
+++ b/conda/common/path.py
@@ -42,6 +42,10 @@ def is_path(value):
return re.match(PATH_MATCH_REGEX, value)


+def expanduser(path):
+ return expandvars(path.replace('~', '${CONDA_DIR}'))
+
+
def expand(path):
# if on_win and PY2:
# path = ensure_fs_path_encoding(path)

EOF
END_SUM=$(sha256sum $CONDA_COMMON_PATH | sed -e's/ .*//')
if [ $START_SUM != $END_SUM ]; then
sed -i -e"s/$START_SUM/$END_SUM/" $(find $CONDA_DIR -name paths.json)
else
echo "Unable to patch conda path module!"
return 1
fi
done
}

function pin_conda_package {
CONDA_PACKAGE_NAME=$1
CONDA_PACKAGE_VERSION=$2
echo "Pinning ${CONDA_PACKAGE_NAME} to ${CONDA_PACKAGE_VERSION}"
CONDA_PIN_FILE=$CONDA_DIR/conda-meta/pinned
CONDA_PIN_TMP=$CONDA_DIR/conda-meta/pinned.tmp
touch ${CONDA_PIN_FILE}
cat ${CONDA_PIN_FILE} | grep -v ${CONDA_PACKAGE_NAME} > ${CONDA_PIN_TMP} || true
echo "${CONDA_PACKAGE_NAME} ==${CONDA_PACKAGE_VERSION}" >> ${CONDA_PIN_TMP}
cat ${CONDA_PIN_TMP} | sort > ${CONDA_PIN_FILE}
}

echo ""
echo "Initializing environment"
echo "---------------------------------"
Expand All @@ -246,32 +101,7 @@ export PYTHONNOUSERSITE=1
# Install and setup conda for downloading packages
export PATH=$CONDA_DIR/bin:$PATH:/sbin
(
echo
echo "Installing conda (self contained Python environment with binary package support)"
if [[ ! -e $CONDA_DIR/bin/conda ]]; then
cd $BUILD_DIR
# FIXME: Get the miniconda people to add a "self check" mode
wget --no-verbose --continue https://repo.continuum.io/miniconda/Miniconda3-latest-Linux-x86_64.sh
#wget --continue https://repo.anaconda.com/miniconda/Miniconda3-${CONDA_VERSION}-Linux-x86_64.sh -O Miniconda3-latest-Linux-x86_64.sh
chmod a+x Miniconda3-latest-Linux-x86_64.sh
# -p to specify the install location
# -b to enable batch mode (no prompts)
# -f to not return an error if the location specified by -p already exists
(
export HOME=$CONDA_DIR
./Miniconda3-latest-Linux-x86_64.sh -p $CONDA_DIR -b -f || exit 1
)
fix_conda
conda config --system --add envs_dirs $CONDA_DIR/envs
conda config --system --add pkgs_dirs $CONDA_DIR/pkgs
fi
conda config --system --set always_yes yes
conda config --system --set changeps1 no
pin_conda_package conda ${CONDA_VERSION}
conda update -q conda
fix_conda
conda config --system --add channels timvideos
conda info
install_conda
)

eval $(cd $TOP_DIR; export HDMI2USB_ENV=1; make env || exit 1) || exit 1
Expand Down
Loading