From 54b9fff0172b4093da63463a0184fdb53288a430 Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Fri, 8 Mar 2019 16:34:50 -0800 Subject: [PATCH 1/7] Updating Homebrew Formula to use a virtualenv. This brings us into conformance with the guidelines for how to install Python applications in Homebrew: https://github.com/Homebrew/brew/blob/5eaf4e5c28e7605cd9d8e1631d376b0e46c98624/docs/Python-for-Formula-Authors.md#installing However, it is worth noting that after some internal discussions, it seems that there may be more clever ways to achieve the same levels of isolation. This change is not a philosophic disagreement with those ideas, just a pragmatic way forward until a better way is obvious. --- .../homebrew/docker/formula_template.txt | 36 ++++--------------- 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/scripts/release/homebrew/docker/formula_template.txt b/scripts/release/homebrew/docker/formula_template.txt index c48c8fdacdc..e778670b256 100644 --- a/scripts/release/homebrew/docker/formula_template.txt +++ b/scripts/release/homebrew/docker/formula_template.txt @@ -1,4 +1,6 @@ class AzureCli < Formula + include Language::Python::Virtualenv + desc "Microsoft Azure CLI 2.0" homepage "https://docs.microsoft.com/cli/azure/overview" url "{{ upstream_url }}" @@ -14,12 +16,8 @@ class AzureCli < Formula {{ resources }} def install - xy = Language::Python.major_minor_version "python3" - site_packages = libexec/"lib/python#{xy}/site-packages" - ENV.prepend_create_path "PYTHONPATH", site_packages - ENV.prepend "LDFLAGS", "-L#{Formula["openssl"].opt_lib}" - ENV.prepend "CFLAGS", "-I#{Formula["openssl"].opt_include}" - ENV.prepend "CPPFLAGS", "-I#{Formula["openssl"].opt_include}" + venv = virtualenv_create(libexec, "python3") + venv.pip_install resources # Get the CLI components we'll install components = [ @@ -31,38 +29,16 @@ class AzureCli < Formula ] components += Pathname.glob(buildpath/"src/command_modules/azure-cli-*/") - # Install dependencies - # note: Even if in 'resources', don't include 'futures' as not needed for Python3 - # and causes import errors. See https://github.com/agronholm/pythonfutures/issues/41 - deps = resources.map(&:name).to_set - ["futures"] - deps.each do |r| - resource(r).stage do - system "python3", *Language::Python.setup_install_args(libexec) - end - end - # Install CLI components.each do |item| cd item do - system "python3", *Language::Python.setup_install_args(libexec) + venv.pip_install item end end - # This replaces the `import pkg_resources` namespace imports from upstream - # with empty string as the import is slow and not needed in this environment. - File.open(site_packages/"azure/__init__.py", "w") {} - File.open(site_packages/"azure/cli/__init__.py", "w") {} - File.open(site_packages/"azure/cli/command_modules/__init__.py", "w") {} - File.open(site_packages/"azure/mgmt/__init__.py", "w") {} - (bin/"az").write <<~EOS #!/usr/bin/env bash - export PYTHONPATH="#{ENV["PYTHONPATH"]}" - if command -v python#{xy} >/dev/null 2>&1; then - python#{xy} -m azure.cli \"$@\" - else - python3 -m azure.cli \"$@\" - fi + #{libexec}/bin/python -m azure.cli \"$@\" EOS bash_completion.install "az.completion" => "az" From ebf6a2afb8b5e3cb98aa27d848aae7af13546e36 Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Fri, 8 Mar 2019 16:39:56 -0800 Subject: [PATCH 2/7] Filter out 'jeepney' and 'entrypoints' from homebrew resources This is a workaround for pypa/pip#6222. It should not be a permanent change. --- scripts/release/homebrew/docker/formula_generate.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/release/homebrew/docker/formula_generate.py b/scripts/release/homebrew/docker/formula_generate.py index d6e899af9d5..d524b471752 100644 --- a/scripts/release/homebrew/docker/formula_generate.py +++ b/scripts/release/homebrew/docker/formula_generate.py @@ -61,7 +61,8 @@ def collect_resources() -> str: def resource_filter(name: str) -> bool: - return not name.startswith('azure-cli') and name not in ('futures') + # TODO remove need for any filters and delete this method. + return not name.startswith('azure-cli') and name not in ('futures', 'jeepney', 'entrypoints') def last_bottle_hash(): From 498a0664a4c2b81d1f023fbb1fab77ca7307d5ef Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Fri, 8 Mar 2019 16:43:18 -0800 Subject: [PATCH 3/7] Temporarily enable Homebrew in PR validatation --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 8bc22f24c3f..fed2d4c1b2b 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -196,7 +196,7 @@ jobs: displayName: Build Homebrew Formula dependsOn: BuildPythonWheel - condition: and(succeeded(), in(variables['Build.Reason'], 'IndividualCI', 'BatchedCI', 'Manual')) + condition: succeeded() pool: vmImage: 'ubuntu-16.04' steps: From e85d15d270fc203233abd58866cd418b817f74bf Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Fri, 8 Mar 2019 17:52:05 -0800 Subject: [PATCH 4/7] Copying wheel-identification regex into our package. This allows us to remove our dependency on an older version of the 'wheel' package. Rob Pike has a saying, "A little copy is better than a little dependency". I believe it's very applicable here, and you can here him talk about that philosophy here: https://www.youtube.com/watch?v=PAAkCSZUG1c&t=9m28s --- scripts/release/homebrew/docker/run.sh | 2 +- src/azure-cli-core/azure/cli/core/extension/__init__.py | 2 +- src/azure-cli-core/azure/cli/core/extension/_resolve.py | 2 +- src/azure-cli-core/azure/cli/core/extension/operations.py | 8 +++++++- src/azure-cli-core/setup.py | 2 +- src/command_modules/azure-cli-extension/setup.py | 2 +- 6 files changed, 12 insertions(+), 6 deletions(-) diff --git a/scripts/release/homebrew/docker/run.sh b/scripts/release/homebrew/docker/run.sh index 679abbc07db..e6cefec019b 100755 --- a/scripts/release/homebrew/docker/run.sh +++ b/scripts/release/homebrew/docker/run.sh @@ -2,7 +2,7 @@ root=$(cd $(dirname $0); pwd) -pip install wheel==0.30.0 +pip install wheel pip install -U pip pip install -r $root/requirements.txt pip install azure-cli==$CLI_VERSION -f /mnt/pypi diff --git a/src/azure-cli-core/azure/cli/core/extension/__init__.py b/src/azure-cli-core/azure/cli/core/extension/__init__.py index 22d8e549ac9..b41bdacce00 100644 --- a/src/azure-cli-core/azure/cli/core/extension/__init__.py +++ b/src/azure-cli-core/azure/cli/core/extension/__init__.py @@ -108,7 +108,7 @@ def get_version(self): return self.metadata.get('version') def get_metadata(self): - from wheel.install import WHEEL_INFO_RE + from .operations import WHEEL_INFO_RE from glob import glob if not extension_exists(self.name): return None diff --git a/src/azure-cli-core/azure/cli/core/extension/_resolve.py b/src/azure-cli-core/azure/cli/core/extension/_resolve.py index 2a101c3b65f..f0cd384b587 100644 --- a/src/azure-cli-core/azure/cli/core/extension/_resolve.py +++ b/src/azure-cli-core/azure/cli/core/extension/_resolve.py @@ -2,7 +2,7 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- -from wheel.install import WHEEL_INFO_RE +from .operations import WHEEL_INFO_RE from pkg_resources import parse_version from knack.log import get_logger diff --git a/src/azure-cli-core/azure/cli/core/extension/operations.py b/src/azure-cli-core/azure/cli/core/extension/operations.py index 63a55aecc33..396e95a81b5 100644 --- a/src/azure-cli-core/azure/cli/core/extension/operations.py +++ b/src/azure-cli-core/azure/cli/core/extension/operations.py @@ -5,6 +5,7 @@ from collections import OrderedDict import sys import os +import re import tempfile import shutil import zipfile @@ -14,7 +15,6 @@ from six.moves.urllib.parse import urlparse # pylint: disable=import-error import requests -from wheel.install import WHEEL_INFO_RE from pkg_resources import parse_version from knack.log import get_logger @@ -40,6 +40,12 @@ LIST_FILE_PATH = os.path.join(os.sep, 'etc', 'apt', 'sources.list.d', 'azure-cli.list') LSB_RELEASE_FILE = os.path.join(os.sep, 'etc', 'lsb-release') +WHEEL_INFO_RE = re.compile( + r"""^(?P(?P.+?)(-(?P\d.+?))?) + ((-(?P\d.*?))?-(?P.+?)-(?P.+?)-(?P.+?) + \.whl|\.dist-info)$""", + re.VERBOSE).match + def _run_pip(pip_exec_args): cmd = [sys.executable, '-m', 'pip'] + pip_exec_args + ['-vv', '--disable-pip-version-check', '--no-cache-dir'] diff --git a/src/azure-cli-core/setup.py b/src/azure-cli-core/setup.py index fd46eb1240d..002539ae27c 100644 --- a/src/azure-cli-core/setup.py +++ b/src/azure-cli-core/setup.py @@ -71,7 +71,7 @@ 'requests>=2.20.0', 'six', 'tabulate>=0.7.7', - 'wheel==0.30.0', + 'wheel', 'azure-mgmt-resource==2.1.0' ] diff --git a/src/command_modules/azure-cli-extension/setup.py b/src/command_modules/azure-cli-extension/setup.py index 63c63145383..e66bcb87c0b 100644 --- a/src/command_modules/azure-cli-extension/setup.py +++ b/src/command_modules/azure-cli-extension/setup.py @@ -33,7 +33,7 @@ DEPENDENCIES = [ 'azure-cli-core', 'pip', - 'wheel==0.30.0', + 'wheel', ] with open('README.rst', 'r', encoding='utf-8') as f: From 1fc65b3a3545069bbde8063c2d5f68737e4b0ede Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Mon, 11 Mar 2019 10:15:39 -0700 Subject: [PATCH 5/7] Moving WHEEL_INFO_RE to __init__ --- src/azure-cli-core/azure/cli/core/extension/__init__.py | 8 +++++++- src/azure-cli-core/azure/cli/core/extension/_resolve.py | 3 +-- src/azure-cli-core/azure/cli/core/extension/operations.py | 7 +------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/extension/__init__.py b/src/azure-cli-core/azure/cli/core/extension/__init__.py index b41bdacce00..8fcfa6c1e3c 100644 --- a/src/azure-cli-core/azure/cli/core/extension/__init__.py +++ b/src/azure-cli-core/azure/cli/core/extension/__init__.py @@ -6,6 +6,7 @@ import os import traceback import json +import re from knack.config import CLIConfig from knack.log import get_logger @@ -29,6 +30,12 @@ EXT_METADATA_MAXCLICOREVERSION = 'azext.maxCliCoreVersion' EXT_METADATA_ISPREVIEW = 'azext.isPreview' +WHEEL_INFO_RE = re.compile( + r"""^(?P(?P.+?)(-(?P\d.+?))?) + ((-(?P\d.*?))?-(?P.+?)-(?P.+?)-(?P.+?) + \.whl|\.dist-info)$""", + re.VERBOSE).match + logger = get_logger(__name__) @@ -108,7 +115,6 @@ def get_version(self): return self.metadata.get('version') def get_metadata(self): - from .operations import WHEEL_INFO_RE from glob import glob if not extension_exists(self.name): return None diff --git a/src/azure-cli-core/azure/cli/core/extension/_resolve.py b/src/azure-cli-core/azure/cli/core/extension/_resolve.py index f0cd384b587..478cf435ddc 100644 --- a/src/azure-cli-core/azure/cli/core/extension/_resolve.py +++ b/src/azure-cli-core/azure/cli/core/extension/_resolve.py @@ -2,12 +2,11 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- -from .operations import WHEEL_INFO_RE from pkg_resources import parse_version from knack.log import get_logger -from azure.cli.core.extension import ext_compat_with_cli +from azure.cli.core.extension import ext_compat_with_cli, WHEEL_INFO_RE from azure.cli.core.extension._index import get_index_extensions diff --git a/src/azure-cli-core/azure/cli/core/extension/operations.py b/src/azure-cli-core/azure/cli/core/extension/operations.py index 396e95a81b5..898f2bb6aaf 100644 --- a/src/azure-cli-core/azure/cli/core/extension/operations.py +++ b/src/azure-cli-core/azure/cli/core/extension/operations.py @@ -5,7 +5,6 @@ from collections import OrderedDict import sys import os -import re import tempfile import shutil import zipfile @@ -40,11 +39,7 @@ LIST_FILE_PATH = os.path.join(os.sep, 'etc', 'apt', 'sources.list.d', 'azure-cli.list') LSB_RELEASE_FILE = os.path.join(os.sep, 'etc', 'lsb-release') -WHEEL_INFO_RE = re.compile( - r"""^(?P(?P.+?)(-(?P\d.+?))?) - ((-(?P\d.*?))?-(?P.+?)-(?P.+?)-(?P.+?) - \.whl|\.dist-info)$""", - re.VERBOSE).match + def _run_pip(pip_exec_args): From 49178f48dc2ae807b76577514ffc63e6be236bc3 Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Mon, 11 Mar 2019 10:31:54 -0700 Subject: [PATCH 6/7] Fixing PEP8 --- src/azure-cli-core/azure/cli/core/extension/operations.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/extension/operations.py b/src/azure-cli-core/azure/cli/core/extension/operations.py index 898f2bb6aaf..5cfed375a47 100644 --- a/src/azure-cli-core/azure/cli/core/extension/operations.py +++ b/src/azure-cli-core/azure/cli/core/extension/operations.py @@ -21,7 +21,7 @@ from azure.cli.core.util import CLIError, reload_module from azure.cli.core.extension import (extension_exists, get_extension_path, get_extensions, get_extension_modname, get_extension, ext_compat_with_cli, EXT_METADATA_ISPREVIEW, - WheelExtension, DevExtension, ExtensionNotInstalledException) + WheelExtension, DevExtension, ExtensionNotInstalledException, WHEEL_INFO_RE) from azure.cli.core.telemetry import set_extension_management_detail from ._homebrew_patch import HomebrewPipPatch @@ -40,8 +40,6 @@ LSB_RELEASE_FILE = os.path.join(os.sep, 'etc', 'lsb-release') - - def _run_pip(pip_exec_args): cmd = [sys.executable, '-m', 'pip'] + pip_exec_args + ['-vv', '--disable-pip-version-check', '--no-cache-dir'] logger.debug('Running: %s', cmd) From 2f9cc5a77068a76ee397bee2ef2addb99efb6bac Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Mon, 11 Mar 2019 14:53:13 -0700 Subject: [PATCH 7/7] Revert "Temporarily enable Homebrew in PR validatation" This reverts commit 498a0664a4c2b81d1f023fbb1fab77ca7307d5ef. --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index fed2d4c1b2b..8bc22f24c3f 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -196,7 +196,7 @@ jobs: displayName: Build Homebrew Formula dependsOn: BuildPythonWheel - condition: succeeded() + condition: and(succeeded(), in(variables['Build.Reason'], 'IndividualCI', 'BatchedCI', 'Manual')) pool: vmImage: 'ubuntu-16.04' steps: