Modify

Opened 6 years ago

Closed 8 months ago

Last modified 8 months ago

#9800 closed defect (fixed)

setup.py does not declare install_requires dependencies

Reported by: Alex Willmer Owned by: Odd Simon Simonsen
Priority: low Component: XmlRpcPlugin
Severity: normal Keywords: setuptools
Cc: Olemis Lang, Ryan J Ollos, Russ Tyndall, Steffen Hoffmann Trac Release: 0.11

Description

TracXMLRPC requires Trac and Genshi, but it doesn't declare this. The following lines should be added to setup.py

    install_requires=[
        'Genshi',
        'Trac',
        ],

Attachments (0)

Change History (22)

comment:1 Changed 6 years ago by Ryan J Ollos

Cc: Ryan J Ollos added

comment:2 Changed 6 years ago by Russ Tyndall

Cc: Russ Tyndall added

Please see some of the comments on #9793.

The problem I had when using install_requires, is that if I had trac 11 installed but tried to install a plugin that required trac 12 , it would download and install trac12 from pipy (which is probably not what the user indended). In similar cases it would be nice to either prompt or fail outright. It would be nice if the trac 11 plugins required trac11 etc, but I have yet to find a way to do this with setuptools unfortunately.

As an aside, why would you be installing TracXMLRPC without installing trac? Or was this intended as a rapid install process of trac? (It seems odd to me that anyone would have wanted to install trac this way, but reasonable to enforce those requirements (ie fail to install if they were unmet)).

Cheers, Russ

comment:3 in reply to:  2 Changed 6 years ago by Ryan J Ollos

Replying to bobbysmith007:

In similar cases it would be nice to either prompt or fail outright. It would be nice if the trac 11 plugins required trac11 etc, but I have yet to find a way to do this with setuptools unfortunately.

Same here. I've also had other problems (#7986) when trying to enforce dependencies through setup.py.

comment:4 Changed 6 years ago by Odd Simon Simonsen

Seems like the 'Nay's have it. Doing development and any form of work with the source code, it is all too easy to have downloads start. I don't like that either.

(BTW, if using install_requires there is no need to declare both Trac and Genshi as Trac already declares Genshi as a dependency)

In a way the plugin requires Trac, but on the other hand it is perfectly valid to install the plugin and install Trac later - it is technically not required for setup or install, it is only required for later operation.

That said, I have often considered implementing a better solution. Particularly related to versioning. The idea I've considered is this non-obtrusive block at the start of setup.py that just checks versions IF Trac is installed:

import sys
version = '1.1.2'
# Check Trac version if installed
try:
    import trac
    if trac.__version__ < '0.11':
        print "RPC %s requires Trac >= 0.11" % version
        sys.exit(1)
except ImportError:
    pass

comment:5 Changed 6 years ago by Alex Willmer

Thanks for the quick responses. I raised the ticket just because it seemed good metadata to have. I hadn't considered side effects of declaring install_requires, but I see what you mean - our build scripts specifically disable all downloads by setuptools to avoid the same issue.

I proposed Genshi as well as Trac because the source:xmlrpcplugin/trunk/tracrpc/ticket.py explicitly imports it.

comment:6 Changed 6 years ago by Alex Willmer

To clarify - I'm happy for this ticket to be closed wontfix or invalid

comment:7 Changed 6 years ago by Odd Simon Simonsen

Yeah, I know. I'm just keeping it open as a reminder to perhaps try my non-obtrusive check. It would catch a class of errors that result from trying to install plugin with a non-supported Trac version.

Having a plugin automagically install Trac is likely something I'll never do, so in that respect this ticket is a wontfix. Trac should preferably be installed the 'right' way first, before installing the plugins.

comment:8 in reply to:  7 Changed 6 years ago by Ryan J Ollos

Cc: Steffen Hoffmann added

Replying to osimons:

Yeah, I know. I'm just keeping it open as a reminder to perhaps try my non-obtrusive check. It would catch a class of errors that result from trying to install plugin with a non-supported Trac version.

I'm really interested in finding a good solution for this as well, so I'll post here if I find some time to experiment, and look forward to your findings.

comment:9 Changed 5 years ago by Ryan J Ollos

(In [12040]) Refs #9800, #10331: Added minimum required Trac version.

comment:10 in reply to:  4 Changed 5 years ago by Ryan J Ollos

Replying to osimons:

That said, I have often considered implementing a better solution. Particularly related to versioning. The idea I've considered is this non-obtrusive block at the start of setup.py that just checks versions IF Trac is installed:

Do you think there is a significant number of instances in which someone will be building an egg in an environment that Trac is not installed? In [12040], I took that approach of exiting when Trac is not found.

comment:11 Changed 5 years ago by Ryan J Ollos

(In [12057]) Fixes #9963, Refs #9800, #8276, #8803, #9146: Added check in setup.py for minimum required Python version. Python 2.5 is currently required. Later, we'll aim to restore full Python 2.4 compatibility.

comment:12 Changed 5 years ago by Ryan J Ollos

In [12098/ticketdeleteplugin/0.11/ticketdelete/__init__.py], I moved a Trac version check for the TicketDeletePlugin into __init__.py. This keeps the setup.py cleaner and more readable. The function currently only handles the < case, but it would be straightforward to extend it. I'd like to put the code into a module that can be shared around trac-hacks, even if only for the plugins I maintain. One idea is to include this via an svn:external, but I think it would not be included in the downloaded zip files. I'll have to investigate further. Is there any option other than using an svn:external?

comment:13 Changed 4 years ago by Ryan J Ollos

I've recently seen pkg_resources.require used to enforce a plugin version. It might be the simplest way to enforce a Trac version. See ticketteamdispatcherplugin/0.11/setup.py@13579:4.

comment:14 Changed 4 years ago by Odd Simon Simonsen

That seems like a simple and unobtrusive setup check. Nice tip.

The difference between setup vs runtime is of course that the runtime checks will continue to work even if user later updates dependencies (to possibly incompatible versions). Plenty of users update Trac only to find that some installed plugins no longer works correctly.

Other than basic sanity checks I don't see much point in putting effort into using setup.py for requirements and dependencies.

comment:15 Changed 4 years ago by Ryan J Ollos

Keywords: setuptools added

comment:16 in reply to:  14 ; Changed 4 years ago by Ryan J Ollos

Replying to osimons:

The difference between setup vs runtime is of course that the runtime checks will continue to work even if user later updates dependencies (to possibly incompatible versions). Plenty of users update Trac only to find that some installed plugins no longer works correctly.

The runtime check seems to be the way to go: [13849].

comment:17 in reply to:  16 Changed 4 years ago by Steffen Hoffmann

Replying to rjollos:

The runtime check seems to be the way to go: [13849].

In [13859] I've implemented this for TagsPlugin, and it works well.

comment:18 in reply to:  13 Changed 3 years ago by Ryan J Ollos

Replying to rjollos:

I've recently seen pkg_resources.require used to enforce a plugin version. It might be the simplest way to enforce a Trac version. See ticketteamdispatcherplugin/0.11/setup.py@13579:4.

This is also used in Trac: trac:browser:/trunk/trac/web/standalone.py@12788:340#L339.

comment:19 Changed 8 months ago by Ryan J Ollos

Is it okay if I apply the pkg_resources.requires('Trac >= 0.11') suggestions from comment:13 and close this one out?

comment:20 Changed 8 months ago by Odd Simon Simonsen

Sure. Go for it.

comment:21 Changed 8 months ago by Ryan J Ollos

Resolution: fixed
Status: newclosed

In 16278:

XmlRpcPlugin 1.1.6: Require Trac >= 0.11

Fixes #9800.

comment:22 Changed 8 months ago by Ryan J Ollos

I had to put pkg_resources.require before the * imports otherwise a traceback would result that disabled Trac when the requirement wasn't met. This isn't ideal in terms of Python coding style, but I didn't see another way.

Traceback (most recent call last):
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/web/main.py", line 567, in _dispatch_request
    dispatcher.dispatch(req)
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/web/main.py", line 249, in dispatch
    resp = chosen_handler.process_request(req)
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/admin/web_ui.py", line 83, in process_request
    panels, providers = self._get_panels(req)
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/admin/web_ui.py", line 163, in _get_panels
    p = list(provider.get_admin_panels(req) or [])
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/admin/web_ui.py", line 203, in get_admin_panels
    if 'TRAC_ADMIN' in req.perm('admin', 'general/basics'):
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/perm.py", line 562, in has_permission
    return self._has_permission(action, resource)
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/perm.py", line 576, in _has_permission
    check_permission(action, perm.username, resource, perm)
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/perm.py", line 468, in check_permission
    perm)
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/perm.py", line 302, in check_permission
    get_user_permissions(username)
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/perm.py", line 397, in get_user_permissions
    actions = self.get_actions_dict()
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/perm.py", line 365, in get_actions_dict
    for requestor in self.requestors:
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/core.py", line 78, in extensions
    components = [component.compmgr[cls] for cls in classes]
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/core.py", line 204, in __getitem__
    component = cls(self)
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/core.py", line 140, in __call__
    self.__init__()
  File "/Users/rjollos/Documents/Workspace/trac-dev/trac-hacks/xmlrpcplugin/trunk/tracrpc/api.py", line 261, in __init__
    __import__('tracrpc', ['__version__']).__version__))
  File "/Users/rjollos/Documents/Workspace/trac-dev/trac-hacks/xmlrpcplugin/trunk/tracrpc/__init__.py", line 19, in <module>
    pkg_resources.require('Trac >= 1.2')
  File "/Users/rjollos/Documents/Workspace/trac-dev/pve/lib/python2.7/site-packages/pkg_resources/__init__.py", line 968, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/Users/rjollos/Documents/Workspace/trac-dev/pve/lib/python2.7/site-packages/pkg_resources/__init__.py", line 859, in resolve
    raise VersionConflict(dist, req).with_context(dependent_req)
VersionConflict: (Trac 1.0.14.dev0 (/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git), Requirement.parse('Trac>=1.2'))

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Odd Simon Simonsen.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.