From 413a03dfda761ada1b54e753f0669cb14844144b Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 15 Dec 2021 18:49:12 -0800 Subject: [PATCH 1/7] add additional code to force non-py3only packages to py3 --- eng/pipelines/templates/variables/globals.yml | 2 +- eng/tox/tox_helper_tasks.py | 8 +++- eng/tox/verify_whl.py | 47 ++++++++++++------- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/eng/pipelines/templates/variables/globals.yml b/eng/pipelines/templates/variables/globals.yml index a949290954e9..d15dc55f35f9 100644 --- a/eng/pipelines/templates/variables/globals.yml +++ b/eng/pipelines/templates/variables/globals.yml @@ -1,4 +1,4 @@ variables: - PythonVersion: '3.6' + PythonVersion: '3.9' skipComponentGovernanceDetection: true AzureSDKCondaChannel: https://azuresdkconda.blob.core.windows.net/channel1/ \ No newline at end of file diff --git a/eng/tox/tox_helper_tasks.py b/eng/tox/tox_helper_tasks.py index c99bbddb5844..1b8f8f94d43a 100644 --- a/eng/tox/tox_helper_tasks.py +++ b/eng/tox/tox_helper_tasks.py @@ -66,7 +66,13 @@ def setup(*args, **kwargs): name_space = packages[0] logging.info("Namespaces found for package {0}: {1}".format(package_name, packages)) - return package_name, name_space, kwargs["version"] + try: + python_requires = kwargs["python_requires"] + # most do not define this, fall back to what we define as universal + except KeyError as e: + python_requires = ">=2.7" + + return package_name, name_space, kwargs["version"], python_requires def parse_req(req): diff --git a/eng/tox/verify_whl.py b/eng/tox/verify_whl.py index fd1976c200fe..992bc31f47cf 100644 --- a/eng/tox/verify_whl.py +++ b/eng/tox/verify_whl.py @@ -18,6 +18,8 @@ unzip_file_to_directory, ) +from pkg_resources import parse_version + logging.getLogger().setLevel(logging.INFO) # Excluding auto generated applicationinsights and loganalytics @@ -30,9 +32,13 @@ ] +def get_wheel(dist_dir, version): + return glob.glob(os.path.join(dist_dir, "*{}*.whl".format(version)))[0] + + def extract_whl(dist_dir, version): # Find whl for the package - path_to_whl = glob.glob(os.path.join(dist_dir, "*{}*.whl".format(version)))[0] + path_to_whl = get_wheel(dist_dir, version) # Cleanup any existing stale files if any and rename whl file to tar.gz zip_file = path_to_whl.replace(".whl", ".tar.gz") @@ -52,9 +58,7 @@ def verify_whl_root_directory(dist_dir, expected_top_level_module, version): root_folders = os.listdir(extract_location) # check for non 'azure' folder as root folder - non_azure_folders = [ - d for d in root_folders if d != expected_top_level_module and not d.endswith(".dist-info") - ] + non_azure_folders = [d for d in root_folders if d != expected_top_level_module and not d.endswith(".dist-info")] if non_azure_folders: logging.error( "whl has following incorrect directory at root level [%s]", @@ -75,17 +79,11 @@ def cleanup(path): def should_verify_package(package_name): - return ( - package_name not in EXCLUDED_PACKAGES - and "nspkg" not in package_name - and "-mgmt" not in package_name - ) + return package_name not in EXCLUDED_PACKAGES and "nspkg" not in package_name and "-mgmt" not in package_name if __name__ == "__main__": - parser = argparse.ArgumentParser( - description="Verify directories included in whl and contents in manifest file" - ) + parser = argparse.ArgumentParser(description="Verify directories included in whl and contents in manifest file") parser.add_argument( "-t", @@ -107,16 +105,31 @@ def should_verify_package(package_name): # get target package name from target package path pkg_dir = os.path.abspath(args.target_package) - pkg_name, namespace, ver = get_package_details(os.path.join(pkg_dir, "setup.py")) + pkg_name, namespace, ver, python_requires = get_package_details(os.path.join(pkg_dir, "setup.py")) + + top_level_module = namespace.split(".")[0] + wheel_location = get_wheel(args.dist_dir, ver) + + if not os.getenv("OVERRIDE_PYVERSION_CHECK"): + if not python_requires: + logging.info("Your package must define python_requires.") + exit(1) + else: + if not parse_version(python_requires) >= parse_version(): + logging.info( + "The python_requires value of '{}' should instead be at least '>=3.0'.".format(python_requires) + ) + exit(1) - top_level_module = namespace.split('.')[0] + # if python_requires and "2.7" not in SpecifierSet(python_requires): + # if "py2" in wheel_location and not os.getenv('IGNORE_WHEEL_CHECK'): + # logging.info("The package {} is marked with 'python_requires{}'. Check your setup.cfg and ensure that 'universal=1' configuration is not present.".format(pkg_name, python_requires)) + # exit(1) if should_verify_package(pkg_name): logging.info("Verifying root directory in whl for package: [%s]", pkg_name) if verify_whl_root_directory(args.dist_dir, top_level_module, ver): logging.info("Verified root directory in whl for package: [%s]", pkg_name) else: - logging.info( - "Failed to verify root directory in whl for package: [%s]", pkg_name - ) + logging.info("Failed to verify root directory in whl for package: [%s]", pkg_name) exit(1) From 846ed6b9ac50a8337655f284633bf126208509dc Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Fri, 17 Dec 2021 13:26:17 -0800 Subject: [PATCH 2/7] apply black, other updates --- eng/tox/tox_helper_tasks.py | 22 ++++++++++++++++++++++ eng/tox/verify_whl.py | 19 +++++++++++-------- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/eng/tox/tox_helper_tasks.py b/eng/tox/tox_helper_tasks.py index 1b8f8f94d43a..a0c9f81a2dc0 100644 --- a/eng/tox/tox_helper_tasks.py +++ b/eng/tox/tox_helper_tasks.py @@ -154,6 +154,28 @@ def find_sdist(dist_dir, pkg_name, pkg_version): return packages[0] +def trim_spec(incoming_spec): + """ + Parses a valid specification. returns the version part of the text. + EG: ">=2.1.1" -> "2.1.1" + "==3.0.0" -> "3.0.0" + "1.0.0" -> "1.0.0" + """ + idx = 0 + excluded_chars = ['<', '>', '=', '~', '!'] + + + for char in incoming_spec: + # reserved chars pulled form https://www.python.org/dev/peps/pep-0508/#grammar + if char not in excluded_chars: + break + else: + idx += 1 + + return incoming_spec[idx:len(excluded_chars)] + + + def find_whl(whl_dir, pkg_name, pkg_version): # This function will find a whl for given package name if not os.path.exists(whl_dir): diff --git a/eng/tox/verify_whl.py b/eng/tox/verify_whl.py index 992bc31f47cf..87cdf9d01fd4 100644 --- a/eng/tox/verify_whl.py +++ b/eng/tox/verify_whl.py @@ -16,6 +16,7 @@ unzip_sdist_to_directory, move_and_rename, unzip_file_to_directory, + trim_spec, ) from pkg_resources import parse_version @@ -110,21 +111,23 @@ def should_verify_package(package_name): top_level_module = namespace.split(".")[0] wheel_location = get_wheel(args.dist_dir, ver) - if not os.getenv("OVERRIDE_PYVERSION_CHECK"): + if not os.getenv("IGNORE_WHEEL_PYVERSION_CHECK"): if not python_requires: - logging.info("Your package must define python_requires.") + logging.info("Your package must define a setup argument for 'python_requires'.") exit(1) else: - if not parse_version(python_requires) >= parse_version(): + if not parse_version(trim_spec(python_requires)) >= parse_version("3.0"): logging.info( "The python_requires value of '{}' should instead be at least '>=3.0'.".format(python_requires) ) exit(1) - - # if python_requires and "2.7" not in SpecifierSet(python_requires): - # if "py2" in wheel_location and not os.getenv('IGNORE_WHEEL_CHECK'): - # logging.info("The package {} is marked with 'python_requires{}'. Check your setup.cfg and ensure that 'universal=1' configuration is not present.".format(pkg_name, python_requires)) - # exit(1) + if "py2" in wheel_location: + logging.info( + "The package {} is marked with 'python_requires{}', but a universal package was generated. Check your setup.cfg and ensure that 'universal=1' configuration is not present.".format( + pkg_name, python_requires + ) + ) + exit(1) if should_verify_package(pkg_name): logging.info("Verifying root directory in whl for package: [%s]", pkg_name) From cb576c95a8d59c6aa932fee86636309f19a780e2 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Fri, 17 Dec 2021 13:30:13 -0800 Subject: [PATCH 3/7] ensure no fallback to 2.7 --- eng/tox/tox_helper_tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/tox/tox_helper_tasks.py b/eng/tox/tox_helper_tasks.py index a0c9f81a2dc0..84974626ccf4 100644 --- a/eng/tox/tox_helper_tasks.py +++ b/eng/tox/tox_helper_tasks.py @@ -70,7 +70,7 @@ def setup(*args, **kwargs): python_requires = kwargs["python_requires"] # most do not define this, fall back to what we define as universal except KeyError as e: - python_requires = ">=2.7" + python_requires = None return package_name, name_space, kwargs["version"], python_requires From f25db74ed79a3d6833c079171a8b2b8f697caad5 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Fri, 17 Dec 2021 13:30:13 -0800 Subject: [PATCH 4/7] ensure no fallback to 2.7 --- eng/tox/tox_helper_tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/tox/tox_helper_tasks.py b/eng/tox/tox_helper_tasks.py index a0c9f81a2dc0..84974626ccf4 100644 --- a/eng/tox/tox_helper_tasks.py +++ b/eng/tox/tox_helper_tasks.py @@ -70,7 +70,7 @@ def setup(*args, **kwargs): python_requires = kwargs["python_requires"] # most do not define this, fall back to what we define as universal except KeyError as e: - python_requires = ">=2.7" + python_requires = None return package_name, name_space, kwargs["version"], python_requires From a4f72b999bb1d23e6923e67a326f58bcc971c636 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Fri, 17 Dec 2021 13:54:38 -0800 Subject: [PATCH 5/7] laurent feedback --- eng/tox/verify_whl.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/eng/tox/verify_whl.py b/eng/tox/verify_whl.py index 87cdf9d01fd4..f531f1c1807e 100644 --- a/eng/tox/verify_whl.py +++ b/eng/tox/verify_whl.py @@ -112,19 +112,21 @@ def should_verify_package(package_name): wheel_location = get_wheel(args.dist_dir, ver) if not os.getenv("IGNORE_WHEEL_PYVERSION_CHECK"): + error_details_string = "For more information, visit https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/packaging.md." + if not python_requires: - logging.info("Your package must define a setup argument for 'python_requires'.") + logging.info("Your package must define a setup argument for 'python_requires'. {}".format(error_details_string)) exit(1) else: - if not parse_version(trim_spec(python_requires)) >= parse_version("3.0"): + if not parse_version(trim_spec(python_requires)) >= parse_version("3.6"): logging.info( - "The python_requires value of '{}' should instead be at least '>=3.0'.".format(python_requires) + "The python_requires value of '{}' should instead be at least '>=3.0'. {}".format(python_requires, error_details_string) ) exit(1) if "py2" in wheel_location: logging.info( - "The package {} is marked with 'python_requires{}', but a universal package was generated. Check your setup.cfg and ensure that 'universal=1' configuration is not present.".format( - pkg_name, python_requires + "The package {} is marked with 'python_requires{}', but a universal package was generated. Check your setup.cfg and ensure that 'universal=1' configuration is not present or remove the `setup.cfg` entirely.{}".format( + pkg_name, python_requires, error_details_string ) ) exit(1) From 7ef183a191f51acc45f9e15f29717e8867327aa5 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Fri, 17 Dec 2021 14:02:45 -0800 Subject: [PATCH 6/7] patch get_package_details --- eng/tox/create_package_and_install.py | 2 +- eng/tox/import_all.py | 2 +- eng/tox/prep_sphinx_env.py | 2 +- eng/tox/run_apistubgen.py | 2 +- eng/tox/run_pylint.py | 2 +- eng/tox/run_sphinx_apidoc.py | 2 +- eng/tox/run_sphinx_build.py | 2 +- eng/tox/verify_sdist.py | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/eng/tox/create_package_and_install.py b/eng/tox/create_package_and_install.py index 54f65e6c1c6a..c82339dad7a3 100644 --- a/eng/tox/create_package_and_install.py +++ b/eng/tox/create_package_and_install.py @@ -57,7 +57,7 @@ def discover_packages(setuppy_path, args): def discover_prebuilt_package(dist_directory, setuppy_path, package_type): packages = [] - pkg_name, _, version = get_package_details(setuppy_path) + pkg_name, _, version, _ = get_package_details(setuppy_path) if package_type == "wheel": prebuilt_package = find_whl(dist_directory, pkg_name, version) else: diff --git a/eng/tox/import_all.py b/eng/tox/import_all.py index e7c2097bc47b..4a4ea89cf126 100644 --- a/eng/tox/import_all.py +++ b/eng/tox/import_all.py @@ -41,7 +41,7 @@ def should_run_import_all(package_name): # get target package name from target package path pkg_dir = os.path.abspath(args.target_package) - package_name, namespace, _ = get_package_details(os.path.join(pkg_dir, 'setup.py')) + package_name, namespace, _, _ = get_package_details(os.path.join(pkg_dir, 'setup.py')) if should_run_import_all(package_name): # import all modules from current package diff --git a/eng/tox/prep_sphinx_env.py b/eng/tox/prep_sphinx_env.py index 5fe37aefca83..e6f3b30c683a 100644 --- a/eng/tox/prep_sphinx_env.py +++ b/eng/tox/prep_sphinx_env.py @@ -138,7 +138,7 @@ def write_version(site_folder, version): args = parser.parse_args() package_path = os.path.abspath(args.target_package) - package_name, namespace, package_version = get_package_details( + package_name, namespace, package_version, _ = get_package_details( os.path.join(package_path, "setup.py") ) diff --git a/eng/tox/run_apistubgen.py b/eng/tox/run_apistubgen.py index 07a877bb4dd5..9da31514f692 100644 --- a/eng/tox/run_apistubgen.py +++ b/eng/tox/run_apistubgen.py @@ -18,7 +18,7 @@ def get_package_wheel_path(pkg_root): # parse setup.py to get package name and version - pkg_name, _, version = get_package_details(os.path.join(pkg_root, "setup.py")) + pkg_name, _, version, _ = get_package_details(os.path.join(pkg_root, "setup.py")) # Check if wheel is already built and available for current package prebuilt_dir = os.getenv("PREBUILT_WHEEL_DIR") if prebuilt_dir: diff --git a/eng/tox/run_pylint.py b/eng/tox/run_pylint.py index f82b892baef0..7312454afce1 100644 --- a/eng/tox/run_pylint.py +++ b/eng/tox/run_pylint.py @@ -43,7 +43,7 @@ pkg_dir = os.path.abspath(args.target_package) - package_name, namespace, ver = get_package_details(os.path.join(pkg_dir, "setup.py")) + package_name, namespace, ver, _ = get_package_details(os.path.join(pkg_dir, "setup.py")) top_level_module = namespace.split('.')[0] diff --git a/eng/tox/run_sphinx_apidoc.py b/eng/tox/run_sphinx_apidoc.py index c23146846438..b3516c2b56e4 100644 --- a/eng/tox/run_sphinx_apidoc.py +++ b/eng/tox/run_sphinx_apidoc.py @@ -117,7 +117,7 @@ def mgmt_apidoc(working_directory, namespace): package_dir = os.path.abspath(args.package_root) output_directory = os.path.join(target_dir, "unzipped/docgen") - pkg_name, namespace, pkg_version = get_package_details(os.path.join(package_dir, 'setup.py')) + pkg_name, namespace, pkg_version, _ = get_package_details(os.path.join(package_dir, 'setup.py')) if should_build_docs(pkg_name): if is_mgmt_package(pkg_name): diff --git a/eng/tox/run_sphinx_build.py b/eng/tox/run_sphinx_build.py index 61293779d5fa..2d3f0ac30e1e 100644 --- a/eng/tox/run_sphinx_build.py +++ b/eng/tox/run_sphinx_build.py @@ -107,7 +107,7 @@ def sphinx_build(target_dir, output_dir): target_dir = os.path.abspath(args.working_directory) package_dir = os.path.abspath(args.package_root) - package_name, _, pkg_version = get_package_details(os.path.join(package_dir, 'setup.py')) + package_name, _, pkg_version, _ = get_package_details(os.path.join(package_dir, 'setup.py')) if should_build_docs(package_name): sphinx_build(target_dir, output_dir) diff --git a/eng/tox/verify_sdist.py b/eng/tox/verify_sdist.py index 137cc3d60469..d46a74d93cad 100644 --- a/eng/tox/verify_sdist.py +++ b/eng/tox/verify_sdist.py @@ -88,7 +88,7 @@ def verify_sdist(package_dir, dist_dir, version): # get target package name from target package path pkg_dir = os.path.abspath(args.target_package) - pkg_name, _, ver = get_package_details(os.path.join(pkg_dir, "setup.py")) + pkg_name, _, ver, _ = get_package_details(os.path.join(pkg_dir, "setup.py")) if should_verify_package(pkg_name): logging.info("Verifying sdist for package [%s]", pkg_name) From 09e3429d0a7ef1aa0086819a4c164a6221a64685 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Fri, 17 Dec 2021 15:39:52 -0800 Subject: [PATCH 7/7] update spacing in a single error handling case --- eng/tox/verify_whl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/tox/verify_whl.py b/eng/tox/verify_whl.py index f531f1c1807e..eef6319d3d8a 100644 --- a/eng/tox/verify_whl.py +++ b/eng/tox/verify_whl.py @@ -125,7 +125,7 @@ def should_verify_package(package_name): exit(1) if "py2" in wheel_location: logging.info( - "The package {} is marked with 'python_requires{}', but a universal package was generated. Check your setup.cfg and ensure that 'universal=1' configuration is not present or remove the `setup.cfg` entirely.{}".format( + "The package {} is marked with 'python_requires{}', but a universal package was generated. Check your setup.cfg and ensure that 'universal=1' configuration is not present or remove the `setup.cfg` entirely. {}".format( pkg_name, python_requires, error_details_string ) )