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.

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

Build python-click Debian package from version 6.7-4 source to fix CLI autocomplete/suggest #1824

Merged
merged 2 commits into from
Jun 29, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ src/lldpd/*
!src/lldpd/patch/
src/mpdecimal/*
!src/mpdecimal/Makefile
src/python-click/*
!src/python-click/Makefile
src/python3/*
!src/python3/Makefile
src/redis/*
Expand Down
6 changes: 6 additions & 0 deletions files/build_templates/sonic_debian_extension.j2
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@ sudo cp {{platform_common_py2_wheel_path}} $FILESYSTEM_ROOT/$PLATFORM_COMMON_PY2
sudo https_proxy=$https_proxy LANG=C chroot $FILESYSTEM_ROOT pip install $PLATFORM_COMMON_PY2_WHEEL_NAME
sudo rm -rf $FILESYSTEM_ROOT/$PLATFORM_COMMON_PY2_WHEEL_NAME

# Install built Python Click package (and its dependencies via 'apt-get -y install -f')
# Do this before installing sonic-utilities so that it doesn't attempt to install
# an older version as part of its dependencies
sudo dpkg --root=$FILESYSTEM_ROOT -i target/debs/python-click*_all.deb || \
sudo LANG=C DEBIAN_FRONTEND=noninteractive chroot $FILESYSTEM_ROOT apt-get -y install -f

Copy link
Collaborator

@qiluo-msft qiluo-msft Jun 28, 2018

Choose a reason for hiding this comment

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

Prefer

pip install click>=6.7

if possible

Copy link
Contributor Author

@jleveque jleveque Jun 28, 2018

Choose a reason for hiding this comment

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

It's possible, but because sonic-utilities is built as a .deb package, it attempts to install its dependencies as .deb packages, also. We've encountered this in the past. If we install the newer Click via pip, we will wind up with two versions of Click installed on the system. The default Python paths should look in the pip installation location (/usr/local/lib/python2.7/dist-packages) before the apt installation location (/usr/lib/python2.7/dist-packages), so in theory it would work, but it's messy and provides the potential for the wrong version to be used if the paths are not as expected. This solution ensures only the 6.7 version of Click is installed on the system, so we have no need to worry about unexpected behavior.

# Install SONiC Utilities (and its dependencies via 'apt-get -y install -f')
sudo dpkg --root=$FILESYSTEM_ROOT -i target/debs/python-sonic-utilities_*.deb || \
sudo LANG=C DEBIAN_FRONTEND=noninteractive chroot $FILESYSTEM_ROOT apt-get -y install -f
Expand Down
16 changes: 16 additions & 0 deletions rules/python-click.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# python-click package
#
# Python Click versions < 6.7 have a bug which causes bash completion
# functionality to stop working after two sublevels. sonic-utilities depends
# on this package, and the most recent version provided by Debian Jessie and
# Stretch is v6.6. We build version 6.7 from source in order to fix this bug.
# TODO: If we upgrade to a distro which provides a version >= 6.7 we will no
# longer need to build this.

PYTHON_CLICK_VERSION = 6.7-4

export PYTHON_CLICK_VERSION

PYTHON_CLICK = python-click_$(PYTHON_CLICK_VERSION)_all.deb
$(PYTHON_CLICK)_SRC_PATH = $(SRC_PATH)/python-click
SONIC_MAKE_DEBS += $(PYTHON_CLICK)
1 change: 1 addition & 0 deletions slave.mk
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ $(addprefix $(TARGET_PATH)/, $(SONIC_INSTALLERS)) : $(TARGET_PATH)/% : \
$(IGB_DRIVER) \
$(IXGBE_DRIVER) \
$(SONIC_DEVICE_DATA) \
$(PYTHON_CLICK) \
$(SONIC_UTILS) \
$(LIBWRAP) \
$(LIBPAM_TACPLUS) \
Expand Down
14 changes: 11 additions & 3 deletions sonic-slave/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,6 @@ RUN apt-get update && apt-get install -y \
python-netaddr \
python-ipaddr \
python-yaml \
# For sonic utilities
python3-netaddr \
# For lockfile
procmail \
# For gtest
Expand All @@ -213,7 +211,17 @@ RUN apt-get update && apt-get install -y \
linuxdoc-tools \
lynx \
texlive-latex-extra \
texlive-latex-recommended
texlive-latex-recommended\
Copy link
Collaborator

Choose a reason for hiding this comment

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

add one blank

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. Thanks, I somehow missed that.

# For python-click build
python-sphinx \
python-docutils \
python3-all \
python3-setuptools \
python3-sphinx \
python3-docutils \
python3-requests \
python3-pytest \
python3-colorama
Copy link
Collaborator

Choose a reason for hiding this comment

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

some are already installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't hurt to specify them more than once, and this way it's explicit which packages are required by other packages. If in the future we have no need for a package and someone removes its dependencies here, shared dependencies will still be listed under the other packages which require them.


# For linux build
RUN apt-get -y build-dep linux
Expand Down
26 changes: 26 additions & 0 deletions src/python-click/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
.ONESHELL:
SHELL = /bin/bash
.SHELLFLAGS += -e

MAIN_TARGET = python-click_$(PYTHON_CLICK_VERSION)_all.deb

$(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% :
# Remove any stale files
rm -rf ./python-click

# Clone python-click Debian repo
git clone https://salsa.debian.org/debian/python-click

pushd ./python-click

# Reset HEAD to the commit of the proper tag
# NOTE: Using "git checkout <tag_name>" here detaches our HEAD,
# which stg doesn't like, so we use this method instead
git reset --hard debian/$(PYTHON_CLICK_VERSION)

# Build source and Debian packages
dpkg-buildpackage -rfakeroot -b -us -uc -j$(SONIC_CONFIG_MAKE_JOBS)
popd

# Move the newly-built .deb package to the destination directory
mv $* $(DEST)/