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

New warnings on non-editable build #862

Closed
manopapad opened this issue Oct 16, 2023 · 13 comments
Closed

New warnings on non-editable build #862

manopapad opened this issue Oct 16, 2023 · 13 comments
Assignees

Comments

@manopapad
Copy link
Contributor

@bryevdv does anything look concerning?

  -- Installing: /home/mpapadakis/noneditable/legate.core/_skbuild/linux-x86_64-3.10/cmake-install/lib/liblegion_canonical_python.so
  '/home/mpapadakis/noneditable/env/bin/python3' '-m' 'pip' 'install' '--global-option=--cmake-build-dir' '--global-option=/home/mpapadakis/noneditable/legate.core/_skbuild/linux-x86_64-3.10/cmake-build/_deps/legion-build/runtime' '--global-option=--prefix' '--global-option=/home/mpapadakis/noneditable/legate.core/_skbuild/linux-x86_64-3.10/cmake-install' '--ignore-installed' '--root' '/' '--prefix' '/home/mpapadakis/noneditable/env' '.'
  DEPRECATION: --build-option and --global-option are deprecated. pip 24.0 will enforce this behaviour change. A possible replacement is to use --config-settings. Discussion can be found at https://github.com/pypa/pip/issues/11859
  WARNING: Implying --no-binary=:all: due to the presence of --build-option / --global-option.
  /home/mpapadakis/noneditable/env/lib/python3.10/site-packages/setuptools/command/build_py.py:204: _Warning: Package 'legate.core._lib' is absent from the `packages` configuration.
  !!

          ********************************************************************************
          ############################
          # Package would be ignored #
          ############################
          Python recognizes 'legate.core._lib' as an importable package[^1],
          but it is absent from setuptools' `packages` configuration.

          This leads to an ambiguous overall configuration. If you want to distribute this
          package, please make sure that 'legate.core._lib' is explicitly added
          to the `packages` configuration field.

          Alternatively, you can also rely on setuptools' discovery methods
          (for example by using `find_namespace_packages(...)`/`find_namespace:`
          instead of `find_packages(...)`/`find:`).

          You can read more about "package discovery" on setuptools documentation page:

          - https://setuptools.pypa.io/en/latest/userguide/package_discovery.html

          If you don't want 'legate.core._lib' to be distributed and are
          already explicitly excluding 'legate.core._lib' via
          `find_namespace_packages(...)/find_namespace` or `find_packages(...)/find`,
          you can try to use `exclude_package_data`, or `include-package-data=False` in
          combination with a more fine grained `package-data` configuration.

          You can read more about "package data files" on setuptools documentation page:

          - https://setuptools.pypa.io/en/latest/userguide/datafiles.html


          [^1]: For Python, any directory (with suitable naming) can be imported,
                even if it does not contain any `.py` files.
                On the other hand, currently there is no concept of package data
                directory, all directories are treated like packages.
          ********************************************************************************

  !!
WARNING: There was an error checking the latest version of pip.
See below for error
Traceback (most recent call last):
  File "/home/mpapadakis/noneditable/env/lib/python3.10/site-packages/pip/_internal/self_outdated_check.py", line 227, in pip_self_version_check
    upgrade_prompt = _self_version_check_logic(
  File "/home/mpapadakis/noneditable/env/lib/python3.10/site-packages/pip/_internal/self_outdated_check.py", line 188, in _self_version_check_logic
    remote_version_str = state.get(current_time)
  File "/home/mpapadakis/noneditable/env/lib/python3.10/site-packages/pip/_internal/self_outdated_check.py", line 76, in get
    last_check = datetime.datetime.fromisoformat(self._state["last_check"])
ValueError: Invalid isoformat string: '2023-08-30T23:44:26Z'
@manopapad
Copy link
Contributor Author

Full build output:
temp3.txt

@bryevdv
Copy link
Contributor

bryevdv commented Oct 16, 2023

Seems to be known upstream issue: pypa/pip#12338

@manopapad
Copy link
Contributor Author

manopapad commented Oct 16, 2023

What about the other two? --build-option and --global-option are deprecated and Package 'legate.core._lib' is absent from the packages configuration

@bryevdv
Copy link
Contributor

bryevdv commented Oct 17, 2023

DEPRECATION: --build-option and --global-option are deprecated

Based on some replies at pypa/pip#11859 and pypa/setuptools#3896 it seems like this might be a current workaround:

 --config-setting="--global-option=--verbose"  .

which seems pretty unpleasant but at a glance at least it doesn't look like there would be any delicate quoting issues.

I guess a longer term solution would be to try to switch to use build instead, but I am not sure up front how involved that would be.

@bryevdv
Copy link
Contributor

bryevdv commented Oct 17, 2023

Python recognizes 'legate.core._lib' as an importable package

This seem like maybe just a change to find_packages config, e.g. just add legate.core._lib. Right now there is a wildcard but maybe that does not (or recently does not) include "private" underscore dirs by default (speculation at this point)

Edit: that didn't seem to work, but then I actually looked in that dir:

dev310 ❯ ls legate/core/_lib 
CMakeLists.txt  context.pyx  types.pyx

Do any of those actually need to be distributed? It seems the cython build resulting in including them but I am also not sure if that inclusion is "before" or "after" the warning

/home/bryan/anaconda3/envs/dev310/lib/python3.10/site-packages/legate/core/_lib/context.cpython-310-x86_64-linux-gnu.so

Edit2: adding a bogus __init__.py gets rid of the warning, not sure offhand if there would be any other implications from doing that.

@bryevdv
Copy link
Contributor

bryevdv commented Oct 17, 2023

@manopapad regarding the error from the datetime check in pip, this comment suggests setting PIP_DISABLE_PIP_VERSION_CHECK=1 as an immediate workaround (and also maybe a good setting for CI environments in any case)

@bryevdv
Copy link
Contributor

bryevdv commented Oct 17, 2023

Regarding the global-option I don't see where we are setting that ourselves anywhere, and it looks like the immediate preceding command is

  [220/221] cd /home/mpapadakis/noneditable/legate.core/_skbuild/linux-x86_64-3.10/cmake-build && /home/mpapadakis/noneditable/env/bin/cmake -P cmake_install.cmake

so I suspect this issue may actually originating in scikit-build

@manopapad
Copy link
Contributor Author

context.pyx types.pyx
Do any of those actually need to be distributed?

No, AFAIK these should all be used exclusively by cython.

Edit2: adding a bogus init.py gets rid of the warning

Do you mind making a PR with this, and asking @Jacobfaib to verify it's good?

regarding the error from the datetime check in pip, pypa/pip#12357 (comment) suggests setting PIP_DISABLE_PIP_VERSION_CHECK=1 as an immediate workaround

Presumably the bug has been fixed pypa/pip#12357 (comment), so I'm inclined to just leave this alone.

@bryevdv
Copy link
Contributor

bryevdv commented Oct 18, 2023

@manopapad actually for the --global-option warning, those are coming from legion:

dev311 ❯ rg -- "--global"
_deps/legion-build/bindings/python/cmake_install.cmake
90:  execute_process(COMMAND /home/bryan/anaconda3/envs/dev311/bin/python3 -m pip install --global-option=--cmake-build-dir "--global-option=/home/bryan/work/legate.core/_skbuild/linux-x86_64-3.11/cmake-build/_deps/legion-build/runtime" --global-option=--prefix "--global-option=${CMAKE_INSTALL_PREFIX}" --ignore-installed --root / --prefix /home/bryan/anaconda3/envs/dev311 . WORKING_DIRECTORY /home/bryan/work/legate.core/_skbuild/linux-x86_64-3.11/cmake-build/_deps/legion-src/bindings/python COMMAND_ECHO STDOUT)

_deps/legion-src/bindings/python/CMakeLists.txt
75:install(CODE "execute_process(COMMAND ${PYTHON_EXECUTABLE} -m pip install --global-option=--cmake-build-dir \"--global-option=${Legion_BINARY_DIR}/runtime\" --global-option=--prefix \"--global-option=\${CMAKE_INSTALL_PREFIX}\" --ignore-installed ${Legion_PYTHON_EXTRA_INSTALL_ARGS} . WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} COMMAND_ECHO STDOUT)")

As mentioned above apparently the "proper" way to do this now is to further funnel through --config-setting. I don't know any timeline for pip 24.0 but give that is explicitly mentioned as the drop release and we are on 23.x now it is maybe worth fixing up.

cc @lightsighter

@manopapad
Copy link
Contributor Author

You're right, here it is https://gitlab.com/StanfordLegion/legion/-/blob/master/bindings/python/CMakeLists.txt?ref_type=heads#L75.

If you know how to fix this, can you please submit a PR against Legion?

@bryevdv
Copy link
Contributor

bryevdv commented Oct 18, 2023

I've already been looking at a few different approaches but unfortunately no success yet, the build dir is not getting communicated and results in a compilation error for a missing header

@bryevdv
Copy link
Contributor

bryevdv commented Oct 18, 2023

@manopapad @lightsighter I have tried a bunch of various incantations with --config-settings but none have worked. AFAICT it comes down to this bit inside legion/bindings/python/setup.py

# Hack: I can't get initialize/finalize_option to work, so just parse
# the arguments here...
parser = argparse.ArgumentParser()
parser.add_argument('--cmake-build-dir', required=False)
parser.add_argument('--prefix', required=False)
args, unknown = parser.parse_known_args()
sys.argv[1:] = unknown

What I think the case is: to the extent that --config-setting can be used to tunnel --global-setting through to setuptools, that won't happen via command line options to an actually invoked-as-script setup.py (i.e. handling sys.argv here would no longer be effective).

printing sys.argv in the the setup.py seems to confirm this:

 ['setup.py', 'bdist_wheel', '--dist-dir', '/tmp/pip-wheel-4nb3s7fa/.tmp-i2j3txai']

One way to work around this would be to rely on env vars instead of command line options. This is actually what the setuptools folks on DPO Packaging advised me on another project in the last year. I didn't love it, but it did work. Before embarking on that here, however, I guess I'd like to know if there is any expected movement on larger efforts that would make all this irrelevant anyway, i.e. IIRC there was some talk of switching legion to scikit-build or something.

@manopapad
Copy link
Contributor Author

There is an ongoing discussion about transitioning Legion to a more standards-compliant build workflow StanfordLegion/legion#1317, so let's hold off on doing drastic changes.

In that case, I think all the warnings have been sufficiently addressed (or punted), so I'll close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants