Skip to content

Comments

Support for url zip subdirectories#398

Merged
radoering merged 3 commits intopython-poetry:mainfrom
ashnair1:zip-subdir-fix
Sep 2, 2022
Merged

Support for url zip subdirectories#398
radoering merged 3 commits intopython-poetry:mainfrom
ashnair1:zip-subdir-fix

Conversation

@ashnair1
Copy link
Contributor

@ashnair1 ashnair1 commented Jun 10, 2022

@ashnair1 ashnair1 mentioned this pull request Jun 10, 2022
Closed
1 task
@andersk
Copy link

andersk commented Jun 11, 2022

We might as well add this to the file-dependency spec too, for local zip files.

But it seems there’s more to do to in poetry-core to make either URL or file subdirectories actually work:

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

[tool.poetry.dependencies]
zulip = { url = "https://github.com/zulip/python-zulip-api/archive/0.8.2.zip", subdirectory = "zulip" }

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

EOF

$ touch test.py

$ python -m venv /tmp/venv

$ . /tmp/venv/bin/activate

$ pip install .
Processing /tmp/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
  Using cached https://github.com/zulip/python-zulip-api/archive/0.8.2.zip
  Installing build dependencies ... done
  Getting requirements to build wheel ... error
  error: subprocess-exited-with-error
  
  × Getting requirements to build wheel did not run successfully.
  │ exit code: 1
  ╰─> [14 lines of output]
      error: Multiple top-level packages discovered in a flat-layout: ['zulip', 'stubs', 'zulip_bots', 'zulip_botserver', 'packaged_helloworld'].
      
      To avoid accidental inclusion of unwanted files or directories,
      setuptools will not proceed with this build.
      
      If you are trying to create a single distribution with multiple packages
      on purpose, you should not rely on automatic discovery.
      Instead, consider the following options:
      
      1. set up custom discovery (`find` directive with `include` or `exclude`)
      2. use a `src-layout`
      3. explicitly set `py_modules` or `packages` with a list of names
      
      To find more information, look for "package discovery" on setuptools docs.
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× Getting requirements to build wheel did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.

@ashnair1
Copy link
Contributor Author

ashnair1 commented Jun 11, 2022

Could you try changing the url to

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

@andersk
Copy link

andersk commented Jun 11, 2022

Like zulip = {url = "https://github.com/zulip/python-zulip-api/archive/0.8.2.zip#subdirectory=zulip", subdirectory = "zulip"}? That does seem to succeed, but it would be inconsistent with how we store the subdirectory for Git requirements.

(My test case should be self-contained and reproducible with just the 5 commands I showed above, if you want to play with it.)

@ashnair1
Copy link
Contributor Author

ashnair1 commented Jun 11, 2022 •
edited
Loading

Yeah I just wanted to see if the url with the subdirectory fragment works. If so it could be stripped out before adding it to pyproject.toml and the lock file.

@ashnair1 ashnair1 changed the title Add subdirectory property to url-dependency spec Fix for zip subdirectories Jun 14, 2022
ashnair1 added a commit to ashnair1/poetry that referenced this pull request Jun 14, 2022
@ashnair1
Use python-poetry/poetry-core#398
a4d926b
@ashnair1 ashnair1 changed the title Fix for zip subdirectories Fix for url zip subdirectories Jun 15, 2022
@ashnair1
Copy link
Contributor Author

ashnair1 commented Jun 20, 2022

Local tests with the new changes

$ 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.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 30, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

radoering
radoering requested changes Aug 27, 2022
View reviewed changes
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 27, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ashnair1 ashnair1 requested a review from radoering August 27, 2022 19:25
@radoering radoering changed the title Fix for url zip subdirectories Support for url zip subdirectories Sep 2, 2022
@radoering radoering merged commit 424da68 into python-poetry:main Sep 2, 2022
@ashnair1 ashnair1 deleted the zip-subdir-fix branch September 2, 2022 18:47
@andersk
Copy link

andersk commented Sep 2, 2022

Is there any reason not to add this for file dependencies too?

@radoering
Copy link
Member

radoering commented Sep 3, 2022

I didn't want to delay this PR any further and had to think about gracefully handling directory dependencies (because both file dependency and directory dependency use the key path in pyproject.toml). I think a have an idea now. I just have to sort things out (e.g. #463) before drafting a PR.

This was referenced Sep 10, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@radoering radoering radoering approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ashnair1 @andersk @radoering