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

python v2 plugin: support building parts conforming to PEP 518 #4087

Merged
merged 10 commits into from Oct 19, 2023

Conversation

brlin-tw
Copy link
Contributor

@brlin-tw brlin-tw commented Apr 1, 2023

Currently Snapcraft only do a pip install . operation when the setup.py file is found in the part's source tree[1], which is no longer the case with projects adhering to PEP 518[2] as such projects may have a pyproject.toml file in their source tree instead(e.g. OCRmyPDF[3]).

This patch instructs Snapcraft to run the pip install . command when pyproject.toml is found in the source tree, according to the pip pull request that implements the support[4] and the Ubuntu package search results[5] this should be supported since core20.

[1]: Line 143 of snapcraft/python.py at 0e2cf91 · snapcore/snapcraft · GitHub
[2]: PEP 518 – Specifying Minimum Build System Requirements for Python Projects | peps.python.org
[3]: Drop setup.py - use pyproject.toml exclusively · ocrmypdf/OCRmyPDF@16fc520
[4]: Install build dependencies as specified in PEP 518 by takluyver · Pull Request #4144 · pypa/pip
[5]: Ubuntu – Package Search Results -- python3-pip

LP: #2008237

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run make lint? (nope, but the CI tests seems to pass?)
  • Have you successfully run pytest tests/unit? (nope, but the CI tests seems to pass?)

Note that I don't really familiar with Python, and with software testing in general. Any help to fulfill the merge criteria is appreciated.

(CRAFT-2119)

@brlin-tw
Copy link
Contributor Author

brlin-tw commented Apr 1, 2023

The pyright test of the make lint command fails with the following error:

tox run -e pyright
pyright: install_deps> python -I -m pip install -r /home/ubuntu/snapcraft/requirements-devel.txt
.pkg: _optional_hooks> python /home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_wheel> python /home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: build_wheel> python /home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
pyright: install_package_deps> python -I -m pip install attrs catkin-pkg click craft-cli craft-grammar craft-parts craft-providers craft-store cryptography==3.4 gnupg jsonschema==2.5.1 launchpadlib lazr.restfulclient lxml macaroonbakery mypy-extensions overrides progressbar pyelftools pylxd pymacaroons python-apt python-debian pyxdg pyyaml raven requests requests-toolbelt requests-unixsocket 'setuptools<66' simplejson snap-helpers tabulate tinydb toml typing-extensions
pyright: install_package> python -I -m pip install --force-reinstall --no-deps /home/ubuntu/snapcraft/.tox/.tmp/package/2/snapcraft-7.3.1.post1+git21f69936-py3-none-any.whl
pyright: commands[0]> pyright snapcraft tests
No configuration file found.
pyproject.toml file found at /home/ubuntu/snapcraft.
Loading pyproject.toml file at /home/ubuntu/snapcraft/pyproject.toml
Assuming Python platform Linux
Searching for source files
Found 161 source files
pyright 1.1.301
/home/ubuntu/snapcraft/snapcraft/cli.py
  /home/ubuntu/snapcraft/snapcraft/cli.py:289:12 - error: "retcode" is possibly unbound (reportUnboundVariable)
1 error, 0 warnings, 0 informations 
Completed in 7.377sec
pyright: exit 1 (7.56 seconds) /home/ubuntu/snapcraft> pyright snapcraft tests pid=7316
.pkg: _exit> python /home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  pyright: FAIL code 1 (48.40=setup[40.83]+cmd[7.56] seconds)
  evaluation failed :( (48.56 seconds)
make: *** [Makefile:38: test-pyright] Error 1

As no code change is done in snapcraft/cli.py I suppose it is an unrelated issue.

@brlin-tw
Copy link
Contributor Author

brlin-tw commented Apr 1, 2023

The pytest tests/unit command errors in the test_get_extensions_data_dir test:

_________________________________________________________________________________________________________ test_get_extensions_data_dir _________________________________________________________________________________________________________

    def test_get_extensions_data_dir():
>       assert (get_extensions_data_dir() / "desktop").is_dir()
E       AssertionError: assert False
E        +  where False = <bound method Path.is_dir of PosixPath('/home/ubuntu/.venv/snapcraft/share/snapcraft/extensions/desktop')>()
E        +    where <bound method Path.is_dir of PosixPath('/home/ubuntu/.venv/snapcraft/share/snapcraft/extensions/desktop')> = (PosixPath('/home/ubuntu/.venv/snapcraft/share/snapcraft/extensions') / 'desktop').is_dir
E        +      where PosixPath('/home/ubuntu/.venv/snapcraft/share/snapcraft/extensions') = get_extensions_data_dir()

tests/unit/extensions/test_extensions.py:234: AssertionError

which again shouldn't be related to this patch.

@brlin-tw
Copy link
Contributor Author

brlin-tw commented Apr 2, 2023

If I rebase the feature branch to the current tip of the main branch(0e2cf91), the make lint command errors with:

(snapcraft) ubuntu@snapcraft-dev:~/snapcraft$ make lint
tox run -e lint-black
ROOT: will run in automatically provisioned tox, host /home/ubuntu/.venv/snapcraft/bin/python3 is missing [requires (has)]: tox==4.4.6 (4.0.11), tox-ignore-env-name-mismatch==0.2.0.post2
Traceback (most recent call last):
  File "/home/ubuntu/.venv/snapcraft/bin/tox", line 8, in <module>
    sys.exit(run())
  File "/home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/tox/run.py", line 19, in run
    result = main(sys.argv[1:] if args is None else args)
  File "/home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/tox/run.py", line 41, in main
    result = provision(state)
  File "/home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/tox/provision.py", line 124, in provision
    return run_provision(provision_tox_env, state)
  File "/home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/tox/provision.py", line 142, in run_provision
    tox_env: PythonRun = cast(PythonRun, state.envs[name])
  File "/home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/tox/session/env_select.py", line 317, in __getitem__
    return self._defined_envs[item].env
  File "/home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/tox/session/env_select.py", line 181, in _defined_envs
    run_env = self._build_run_env(name)
  File "/home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/tox/session/env_select.py", line 242, in _build_run_env
    runner = REGISTER.runner(cast(str, env_conf["runner"]))
  File "/home/ubuntu/.venv/snapcraft/lib/python3.8/site-packages/tox/tox_env/register.py", line 72, in runner
    return self._run_envs[name]
KeyError: 'ignore_env_name_mismatch'
make: *** [Makefile:14: test-black] Error 1

The pytest tests/unit command errors with the same test:

_________________________________________________________________________________________________________ test_get_extensions_data_dir _________________________________________________________________________________________________________

    def test_get_extensions_data_dir():
>       assert (get_extensions_data_dir() / "desktop").is_dir()
E       AssertionError: assert False
E        +  where False = <bound method Path.is_dir of PosixPath('/home/ubuntu/.venv/snapcraft/share/snapcraft/extensions/desktop')>()
E        +    where <bound method Path.is_dir of PosixPath('/home/ubuntu/.venv/snapcraft/share/snapcraft/extensions/desktop')> = (PosixPath('/home/ubuntu/.venv/snapcraft/share/snapcraft/extensions') / 'desktop').is_dir
E        +      where PosixPath('/home/ubuntu/.venv/snapcraft/share/snapcraft/extensions') = get_extensions_data_dir()

tests/unit/extensions/test_extensions.py:234: AssertionError

@brlin-tw brlin-tw changed the title [WIP] python v2 plugin: support building parts conforming to PEP 518 python v2 plugin: support building parts conforming to PEP 518 Apr 2, 2023
Currently Snapcraft only do a `pip install .` operation when the
setup.py file is found in the part's source tree[1], which is no longer
the case with projects adhering to PEP 518[2] as such projects may have
a pyproject.toml file in their source tree instead(e.g. OCRmyPDF[3]).

This patch instructs Snapcraft to run the `pip install .` command when
pyproject.toml is found in the source tree, according to the pip pull
request that implements the support[4] and the Ubuntu package search
results[5] this should be supported since core20.

[1]: [Line 143 of snapcraft/python.py at 0e2cf91 · snapcore/snapcraft · GitHub](https://github.com/snapcore/snapcraft/blob/0e2cf91/snapcraft_legacy/plugins/v2/python.py#L143)
[2]: [PEP 518 – Specifying Minimum Build System Requirements for Python Projects | peps.python.org](https://peps.python.org/pep-0518/)
[3]: [Drop setup.py - use pyproject.toml exclusively · ocrmypdf/OCRmyPDF@16fc520](ocrmypdf/OCRmyPDF@16fc520)
[4]: [Install build dependencies as specified in PEP 518 by takluyver · Pull Request canonical#4144 · pypa/pip](pypa/pip#4144)
[5]: [Ubuntu – Package Search Results -- python3-pip](https://packages.ubuntu.com/search?lang=en&suite=all&exact=1&searchon=names&keywords=python3-pip)

LP: #2008237

Signed-off-by: 林博仁(Buo-ren Lin) <Buo.Ren.Lin@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2023

Codecov Report

Merging #4087 (095bc86) into main (bf37fd4) will not change coverage.
The diff coverage is 100.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@           Coverage Diff           @@
##             main    #4087   +/-   ##
=======================================
  Coverage   89.17%   89.17%           
=======================================
  Files         321      321           
  Lines       21608    21608           
=======================================
  Hits        19268    19268           
  Misses       2340     2340           
Files Coverage Δ
snapcraft_legacy/plugins/v2/python.py 96.87% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@sergiusens sergiusens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brlin-tw
Copy link
Contributor Author

brlin-tw commented Apr 5, 2023

@toabctl
Copy link
Member

toabctl commented May 2, 2023

@sergiusens @brlin-tw I did the work for craft-parts. See canonical/craft-parts#435

@sanjacob
Copy link

sanjacob commented Oct 2, 2023

@toabctl I am still not able to use pyproject.toml, did you change go through? I am using snapcraft 7.5.2

@toabctl
Copy link
Member

toabctl commented Oct 2, 2023

@toabctl I am still not able to use pyproject.toml, did you change go through? I am using snapcraft 7.5.2

Did you use core22 as base? That should work.

@sanjacob
Copy link

sanjacob commented Oct 2, 2023

It did not work, I had to use a workaround using dephell which did.

@mr-cal
Copy link
Collaborator

mr-cal commented Oct 17, 2023

@brlin-tw, do you mind if I add a spread test and fix the linter failures for this PR?

Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
@brlin-tw
Copy link
Contributor Author

@brlin-tw, do you mind if I add a spread test and fix the linter failures for this PR?

You are free to do so, thanks for helping!

Do let me know if there's anything I need to cooperate of.

Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
@mr-cal mr-cal mentioned this pull request Oct 18, 2023
4 tasks
@mr-cal
Copy link
Collaborator

mr-cal commented Oct 18, 2023

Linter error is fixed in #4406.

@mr-cal mr-cal requested a review from a team October 18, 2023 13:56
@mr-cal mr-cal requested a review from cmatsuoka October 18, 2023 18:57
@mr-cal mr-cal merged commit 2d59662 into canonical:main Oct 19, 2023
9 checks passed
@mr-cal mr-cal deleted the fix-lp-2008237 branch October 19, 2023 15:26
Hook25 pushed a commit to Hook25/snapcraft that referenced this pull request Nov 7, 2023
…ical#4087)

LP: #2008237

Signed-off-by: 林博仁(Buo-ren Lin) <Buo.Ren.Lin@gmail.com>
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
Co-authored-by: Callahan Kovacs <callahan.kovacs@canonical.com>
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

Successfully merging this pull request may close these issues.

None yet

8 participants