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

Make tox a standard package #30416

Closed
mkoeppe opened this issue Aug 21, 2020 · 44 comments
Closed

Make tox a standard package #30416

mkoeppe opened this issue Aug 21, 2020 · 44 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 21, 2020

see #30410.

Discussion/vote at sage-devel.

Depends on #31699

CC: @jhpalmieri @tobiasdiez @fchapoton @slel @orlitzky

Component: packages: standard

Author: Matthias Koeppe

Branch: 65e6330

Reviewer: John Palmieri

Issue created by migration from https://trac.sagemath.org/ticket/30416

@mkoeppe mkoeppe added this to the sage-9.2 milestone Aug 21, 2020
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 9, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 9, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 9, 2020

New commits:

f8a7ee6build/pkgs/tox: Make it a standard package
471ab84build/pkgs/tox/SPKG.rst: New
6913cd8build/pkgs/tox/dependencies: New

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 9, 2020

Commit: 6913cd8

@jhpalmieri
Copy link
Member

comment:5

Should there also be an spkg-configure.m4 file? (As a policy issue, should all new standard packages be encouraged, required if at all possible, to have such files, to try to maintain the momentum toward working well with distros?)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 9, 2020

comment:6

That's a good idea, and I would support adding this policy too.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 9, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

3b510ddbuild/pkgs/tox/spkg-configure.m4: New

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 9, 2020

Changed commit from 6913cd8 to 3b510dd

@jhpalmieri
Copy link
Member

comment:9

Replying to @mkoeppe:

That's a good idea, and I would support adding this policy too.

See #30543 for documentation for spkg-configure.m4 and the statement of this policy.

@jhpalmieri
Copy link
Member

comment:10

We're missing some dependencies:

install_requires = 
	filelock>=3.0.0
	packaging>=14
	pluggy>=0.12.0
	py>=1.4.17
	six>=1.14.0 # required when virtualenv>=20
	toml>=0.9.4
        virtualenv!=20.0.0,!=20.0.1,!=20.0.2,!=20.0.3,!=20.0.4,!=20.0.5,!=20.0.6,!=20.0.7,>=16.0.0
	colorama>=0.4.1 ;platform_system=="Windows"
	importlib-metadata>=0.12,<2;python_version<"3.8"

When I run ./sage --tox --version, I get

ModuleNotFoundError: No module named 'pluggy'

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 10, 2020

comment:11

Indeed, thanks for catching this.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 10, 2020

Dependencies: #30544

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Sep 10, 2020
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 10, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 10, 2020

Changed commit from 3b510dd to f8a7ee6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 2, 2021

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

6f3b864build/pkgs/[s-t]*/SPKG.rst: Reformat title in format 'spkg_name: Short description'
23558f8build/pkgs/[t-z]*/SPKG.rst: Reformat title in format 'spkg_name: Short description' -- completes coverage of all standard packages
17789dfbuild/pkgs/[z-z]*/SPKG.rst: Reformat title in format 'spkg_name: Short description' -- completes coverage of all optional packages
944408dbuild/pkgs/[a-z]*/SPKG.rst: Reformat title in format 'spkg_name: Short description' -- completes coverage of all experimental packages
feafa60src/doc/en/installation/standard_packages.rst: Remove
58c63e1src/doc/bootstrap: Add brief explanation of the standard/optional/experimental package types
54d4fb1extra line after version
565dd46Merge branch 't/29655/improve_build_pkgs___spkg_rst' into t/30416/make_tox_a_standard_package
7f4c0b2build/pkgs/{distlib,appdirs,importlib_resources}: Add dependencies of virtualenv
b51cf6bbuild/pkgs/virtualenv/dependencies: Add deps

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 2, 2021

Changed dependencies from #30544 to #29655

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Mar 20, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 27, 2021

Changed dependencies from #29655 to #31699

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 27, 2021

Changed commit from b51cf6b to cc2f4ae

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 27, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

012774fbuild/pkgs/packaging: Update to 20.9
fff89bcbuild/pkgs/importlib_metadata: Update to 4.0.1
e14a5edbuild/pkgs/pip: Update to 21.1.1
3386fa1build/pkgs/toml: New
71aeec7build/pkgs/typing_extensions: New
ecdf3e1build/pkgs/importlib_metadata/dependencies: Update
2e74014Merge #31699
e694b15build/pkgs/tox: Make it a standard package
9c7a1c2build/pkgs: Add tox dependencies: filelock py virtualenv pluggy
cc2f4aebuild/pkgs/{distlib,appdirs,importlib_resources}: Add dependencies of virtualenv

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 27, 2021

Changed commit from cc2f4ae to b9c4ec8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 27, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

5488ecdbuild/pkgs: Update tox and dependencies to latest
b9c4ec8build/pkgs/packaging/checksums.ini: Add upstream_url

@jhpalmieri
Copy link
Member

comment:26

py doesn't build for me; even if it did, it looks like it's trying to download things at the start of its build process. Log attached.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 28, 2021

comment:27

Attachment: py-1.10.0.log

Branch pushed to git repo; I updated commit sha1. New commits:

ef12a3ebuild/pkgs/pkgconfig/checksums.ini: Add upstream_url
7a6d0b6build/pkgs/packaging/checksums.ini: Add upstream_url
6abfe84Merge #31699
ce2ec11build/pkgs/py/dependencies: Add setuptools_scm (a setup-requires)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 28, 2021

Changed commit from b9c4ec8 to ce2ec11

@jhpalmieri
Copy link
Member

comment:28

It's a little unfortunate that the top-level configure recognizes that

configure:38013: checking for tox >= 2.5.0
configure:38096: result: /usr/local/bin/tox
configure:38107: will use system package and not install SPKG tox

but our build system isn't smart enough to skip its dependencies.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 28, 2021

comment:29

Great point! I'll see what I can do about this

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 29, 2021

Changed commit from ce2ec11 to 65e6330

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 29, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

65e6330build/pkgs{appdirs,distlib,filelock,virtualenv}/spkg-configure.m4: Mark as not required if system tox is used

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 29, 2021

comment:31

Here's something that will avoid installing 4 packages in this case:

  • appdirs, distlib, filelock, virtualenv

I haven't added the same logic to the other packages:

@jhpalmieri
Copy link
Member

comment:32

Okay, thanks, looks good.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 31, 2021

comment:33

Thanks!

@vbraun
Copy link
Member

vbraun commented Jun 20, 2021

Changed branch from u/mkoeppe/make_tox_a_standard_package to 65e6330

@kliem
Copy link
Contributor

kliem commented Jun 22, 2021

comment:35

Replying to @jhpalmieri:

py doesn't build for me; even if it did, it looks like it's trying to download things at the start of its build process. Log attached.

What was the resolution to this?

I'm getting build failure of sage because of this now. It's not terrible and I guess py isn't needed by anything. But I'm also wondering, why it is a dependency of tox and pytest in the first place. Neither of them lists py as a dependency. Where do we use it?

@kliem
Copy link
Contributor

kliem commented Jun 22, 2021

Changed commit from 65e6330 to none

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 22, 2021

comment:36

Replying to @kliem:

Replying to @jhpalmieri:

py doesn't build for me; even if it did, it looks like it's trying to download things at the start of its build process. Log attached.

What was the resolution to this?

The missing dependency on setuptools_scm was added

I'm getting build failure of sage because of this now. It's not terrible and I guess py isn't needed by anything. But I'm also wondering, why it is a dependency of tox and pytest in the first place. Neither of them lists py as a dependency. Where do we use it?

It's an install_requires of tox: https://github.com/tox-dev/tox/blob/master/setup.cfg

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

No branches or pull requests

4 participants