Fix for url zip subdirectories

[ashnair1]

Added support for parsing subdirectory param from zip urls

i.e. support the following

poetry add 'https://github.com/foo/bar/archive/0.1.0.zip#subdirectory=baz'

• Added tests for changed code.

Requires: python-poetry/poetry-core#398

Original request was raised here -> #5172 (comment)

cc: @andersk @neersighted @abn

[andersk]

The poetry add command succeeds, but breaks all future poetry commands when they try to parse the configuration.

$ poetry init -n

$ poetry add 'https://github.com/zulip/python-zulip-api/archive/0.8.2.zip#subdirectory=zulip'

$ poetry shell

The Poetry configuration is invalid:
  - [dependencies.zulip] {'url': 'https://github.com/zulip/python-zulip-api/archive/0.8.2.zip#subdirectory=zulip', 'subdirectory': 'zulip'} is not valid under any of the given schemas

[ashnair1]

The subdirectory param needs to be added to the poetry-schema in poetry-core for this to work. Opened python-poetry/poetry-core#398 to address this.

[ashnair1]

This requires #5172 and python-poetry/poetry-core#398 for tests to pass. Former adds subdirectory support for locker (5e8b9ef) and the latter is required for UrlDependency with subdirectory support and schema updates.

[ashnair1]

Local tests

$ cat pyproject.toml 
[tool.poetry]
name = "test"
version = "0.1.0"
description = ""
authors = []

[tool.poetry.dependencies]
python="^3.9"

[build-system]
requires = ["poetry-core @ https://github.com/python-poetry/poetry-core/archive/refs/pull/398/head.zip"]
build-backend = "poetry.core.masonry.api"

$ poetry add 'https://github.com/zulip/python-zulip-api/archive/0.8.2.zip#subdirectory=zulip'
Creating virtualenv test in /media/ashwin/DATA2/poetry-test/.venv

Updating dependencies
Resolving dependencies... (15.8s)

Writing lock file

Package operations: 10 installs, 0 updates, 0 removals

  • Installing certifi (2022.5.18.1)
  • Installing charset-normalizer (2.0.12)
  • Installing idna (3.3)
  • Installing urllib3 (1.26.9)
  • Installing requests (2.28.0)
  • Installing click (8.1.3)
  • Installing distro (1.7.0)
  • Installing matrix-client (0.4.0)
  • Installing typing-extensions (4.2.0)
  • Installing zulip (0.8.2 https://github.com/zulip/python-zulip-api/archive/0.8.2.zip)

$ touch test.py

$ rm -rf /tmp/venv/

$ python -m venv /tmp/venv

$ . /tmp/venv/bin/activate

$ pip install .
Processing /media/ashwin/DATA2/poetry-test
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Collecting zulip@ https://github.com/zulip/python-zulip-api/archive/0.8.2.zip#subdirectory=zulip
  Using cached https://github.com/zulip/python-zulip-api/archive/0.8.2.zip (2.7 MB)
  Preparing metadata (setup.py) ... done
Collecting requests[security]>=0.12.1
  Using cached requests-2.28.0-py3-none-any.whl (62 kB)
Collecting matrix_client
  Using cached matrix_client-0.4.0-py2.py3-none-any.whl (43 kB)
Collecting distro
  Using cached distro-1.7.0-py3-none-any.whl (20 kB)
Collecting click
  Using cached click-8.1.3-py3-none-any.whl (96 kB)
Collecting typing_extensions>=3.7
  Using cached typing_extensions-4.2.0-py3-none-any.whl (24 kB)
Collecting urllib3<1.27,>=1.21.1
  Using cached urllib3-1.26.9-py2.py3-none-any.whl (138 kB)
Collecting charset-normalizer~=2.0.0
  Using cached charset_normalizer-2.0.12-py3-none-any.whl (39 kB)
Collecting idna<4,>=2.5
  Using cached idna-3.3-py3-none-any.whl (61 kB)
Collecting certifi>=2017.4.17
  Using cached certifi-2022.5.18.1-py3-none-any.whl (155 kB)
Using legacy 'setup.py install' for zulip, since package 'wheel' is not installed.
Building wheels for collected packages: test
  Building wheel for test (pyproject.toml) ... done
  Created wheel for test: filename=test-0.1.0-py3-none-any.whl size=975 sha256=941e48022c674808a6b6538eecc26f5ba71b212c2a5c48aad6c0b9b4ec55ecae
  Stored in directory: /home/ashwin/.cache/pip/wheels/f1/61/6f/a7f6d6a2bbb78c43b2195dbbbe1b7697a94a999f4e8d5a11a0
Successfully built test
Installing collected packages: urllib3, typing_extensions, idna, distro, click, charset-normalizer, certifi, requests, matrix_client, zulip, test
  Running setup.py install for zulip ... done
Successfully installed certifi-2022.5.18.1 charset-normalizer-2.0.12 click-8.1.3 distro-1.7.0 idna-3.3 matrix_client-0.4.0 requests-2.28.0 test-0.1.0 typing_extensions-4.2.0 urllib3-1.26.9 zulip-0.8.2
WARNING: You are using pip version 22.0.4; however, version 22.1.2 is available.
You should consider upgrading via the '/tmp/venv/bin/python -m pip install --upgrade pip' command.

[ashnair1]

Dropped link to #5172 by adding locker subdirectory support

[ashnair1]

@andersk Ran some local tests above and looks like the fix works.

[neersighted]

@ashnair1 Any chance you can rebase this? Might be able to sneak it into 1.2, or if not we'll scope it for the follow-up release that should hopefully be soon after.

[ashnair1]

Tests should pass once python-poetry/poetry-core#398 is merged

[radoering]

I'm sorry to be a killjoy, but there are still some uncertainties in this PR (plus the required core PR) so that it will probably not make it into 1.2.

However, we are planning to just release 1.3 if there are some new features (and not if one specific big feature is ready).

I just skimmed over but it seems the test coverage could be improved. Maybe, some similar tests as in #5172 can be implemented.

[radoering]

Setting the subdirectory as class attribute seems like a crude hack to me. I think that has to be refactored.

[neersighted]

Hey @ashnair1 -- we'd like to land this for 1.3 which is creeping up on us. Could you address @radoering's review so we can unstuck this?

[ashnair1]

Hey @ashnair1 -- we'd like to land this for 1.3 which is creeping up on us. Could you address @radoering's review so we can unstuck this?

I've added some additional tests. Haven't figured out why it fails on Windows 3.8 and macOS 3.8 yet.

Regarding the refactor, it's been a while since I looked at the codebase and I'm currently a bit swamped at work, so it will take me some time to do this. If the release is indeed around the corner, I welcome someone more intimate with the codebase to take a crack at it. Otherwise, I might have some time to look into this towards the end of next week.

[ashnair1]

I'm not sure why but for some tests, 'subdirectory': 'subdir' gets added when it's not supposed to. The random nature of this error might be due to a particular test being run before another that creates this whole issue. Can't seem to narrow down what's causing this though.

[radoering]

That's probably a side effect of setting a class attribute. cf #5811 (comment)