[2/2] Remove pwclient

Let's start managing this via a separate project, which will allow the
client to evolve separately from the server. No redirect is added for
the old '/pwclient' URL as it seems wiser to return a HTTP 404 error
code.

Signed-off-by: Stephen Finucane <stephen@that.guru>
---
 docs/deployment/installation.rst              |   4 +-
 docs/usage/clients.rst                        |  41 +-
 docs/usage/design.rst                         |   4 +-
 patchwork/bin/pwclient                        | 812 ------------------
 patchwork/templates/patchwork/project.html    |   3 +-
 patchwork/urls.py                             |   2 -
 patchwork/views/pwclient.py                   |   8 -
 .../remove-pwclient-2ad030cdc1425e80.yaml     |   8 +
 tools/patchwork-update-commits                |   2 +-
 tools/post-receive.hook                       |   6 +-
 tox.ini                                       |   2 +-
 11 files changed, 36 insertions(+), 856 deletions(-)
 delete mode 100755 patchwork/bin/pwclient
 create mode 100644 releasenotes/notes/remove-pwclient-2ad030cdc1425e80.yaml

Stephen Finucane <stephen@that.guru> writes:

> Let's start managing this via a separate project, which will allow the
> client to evolve separately from the server. No redirect is added for
> the old '/pwclient' URL as it seems wiser to return a HTTP 404 error
> code.

I can't say I'm thrilled by the idea. It adds complexity and I don't
know what it would make easier. Presumably patches for it would still go
to this list and be reviewed in the usual way, so I'm not sure how it
would help workflow. It's also a single file at the moment, so it's not
like it's so complex that it should be spun out to simplify things...

Do you have a plan to avoid losing the git history if you do this move?

I am somewhat biased because I almost always just invoke pwclient from
the project directory as patchwork/bin/pwclient, so this would totally
break my usecase, but I realise that's not a valid reason per se.

Regards,
Daniel

>
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> ---
>  docs/deployment/installation.rst              |   4 +-
>  docs/usage/clients.rst                        |  41 +-
>  docs/usage/design.rst                         |   4 +-
>  patchwork/bin/pwclient                        | 812 ------------------
>  patchwork/templates/patchwork/project.html    |   3 +-
>  patchwork/urls.py                             |   2 -
>  patchwork/views/pwclient.py                   |   8 -
>  .../remove-pwclient-2ad030cdc1425e80.yaml     |   8 +
>  tools/patchwork-update-commits                |   2 +-
>  tools/post-receive.hook                       |   6 +-
>  tox.ini                                       |   2 +-
>  11 files changed, 36 insertions(+), 856 deletions(-)
>  delete mode 100755 patchwork/bin/pwclient
>  create mode 100644 releasenotes/notes/remove-pwclient-2ad030cdc1425e80.yaml
>
> diff --git a/docs/deployment/installation.rst b/docs/deployment/installation.rst
> index c086d9a8..d422573d 100644
> --- a/docs/deployment/installation.rst
> +++ b/docs/deployment/installation.rst
> @@ -657,8 +657,8 @@ This sample hook has support to update patches to different states depending on
>  which branch is being pushed to. See the ``STATE_MAP`` setting in that file.
>  
>  If you are using a system other than Git, you can likely write a similar hook
> -using ``pwclient`` to update patch state. If you do write one, please
> -contribute it.
> +using the :doc:`APIs </api/index>` or :doc:`API clients </usage/clients>` to to
> +update patch state. If you do write one, please contribute it.
>  
>  
>  .. _deployment-cron:
> diff --git a/docs/usage/clients.rst b/docs/usage/clients.rst
> index a131fc8d..57c8a1a1 100644
> --- a/docs/usage/clients.rst
> +++ b/docs/usage/clients.rst
> @@ -4,44 +4,39 @@ Clients
>  A number of clients are available for interacting with Patchwork's various
>  APIs.
>  
> +
>  pwclient
>  --------
>  
> -The `pwclient` application, provided with Patchwork, can be used to interact
> -with Patchwork from the command line. Functionality provided by `pwclient`
> -includes:
> +.. versionchanged:: 2.2
> +
> +   :program:`pwclient` was previously provided with Patchwork. It has been
> +   packaged as a separate application since Patchwork v2.2.0.
> +
> +The :program:`pwclient` application can be used to interact with Patchwork from
> +the command line. Functionality provided by :program:`pwclient` includes:
>  
>  - Listing patches, projects, and checks
>  - Downloading and applying patches to a local code base
>  - Modifying the status of patches
>  - Creating new checks
>  
> -`pwclient` can be downloaded from the `Ozlabs Patchwork instance`__, or at the
> -following path for most other Patchwork instances:
> -
> -    http://patchwork.example.com/pwclient/
> -
> -where `patchwork.example.com` corresponds to the URL a Patchwork instance is
> -hosted at.
> -
> -Once downloaded, view information about all the operations supported by
> -`pwclient`, run:
> -
> -.. code-block:: shell
> +More information on :program:`pwclient`, including installation and usage
> +instructions, can be found in the `documentation`__ and the `GitHub repo`__.
>  
> -    $ pwclient --help
> +__ https://pwclient.readthedocs.io/
> +__ https://github.com/getpatchwork/pwclient/
>  
> -__ https://patchwork.ozlabs.org/pwclient/
>  
>  git-pw
>  ------
>  
> -The `git-pw` application can be used to integrate Git with Patchwork. The
> -`git-pw` application relies on the REST API and can be used to interact to
> -list, download and apply series, bundles and individual patches.
> +The :program:`git-pw` application can be used to integrate Git with Patchwork.
> +The :program:`git-pw` application relies on the REST API and can be used to
> +interact to list, download and apply series, bundles and individual patches.
>  
> -More information on `git-pw`, including installation and usage instructions,
> -can be found in the `documentation`__ and the `GitHub repo`__.
> +More information on :program:`git-pw`, including installation and usage
> +instructions, can be found in the `documentation`__ and the `GitHub repo`__.
>  
>  __ https://git-pw.readthedocs.io/
> -__ https://github.com/getpatchwork/git-pw
> +__ https://github.com/getpatchwork/git-pw/
> diff --git a/docs/usage/design.rst b/docs/usage/design.rst
> index 56719eed..05cf0b35 100644
> --- a/docs/usage/design.rst
> +++ b/docs/usage/design.rst
> @@ -22,5 +22,5 @@ Patchwork users shouldn't require a specific version control system
>  Not everyone uses git for kernel development, and not everyone uses git for
>  Patchwork-tracked projects.
>  
> -It's still possible to hook other programs into Patchwork, using the pwclient
> -command-line client for Patchwork, or directly to the XML RPC interface.
> +It's still possible to hook other programs into Patchwork, using various
> +:doc:`clients </usage/clients>` or the :doc:`APIs </api/index>` directly.
> diff --git a/patchwork/bin/pwclient b/patchwork/bin/pwclient
> deleted file mode 100755
> index 2cff3a35..00000000
> --- a/patchwork/bin/pwclient
> +++ /dev/null
> @@ -1,812 +0,0 @@
> -#!/usr/bin/env python
> -# -*- coding: utf-8 -*-
> -#
> -# Patchwork command line client
> -# Copyright (C) 2008 Nate Case <ncase@xes-inc.com>
> -#
> -# SPDX-License-Identifier: GPL-2.0-or-later
> -
> -from __future__ import print_function
> -from __future__ import unicode_literals
> -
> -import os
> -import sys
> -try:
> -    import xmlrpclib
> -except ImportError:
> -    # Python 3 has merged/renamed things.
> -    import xmlrpc.client as xmlrpclib
> -import argparse
> -import string
> -import subprocess
> -try:
> -    import ConfigParser
> -except ImportError:
> -    # Python 3 has renamed things.
> -    import configparser as ConfigParser
> -import shutil
> -import re
> -import io
> -
> -
> -# Default Patchwork remote XML-RPC server URL
> -# This script will check the PW_XMLRPC_URL environment variable
> -# for the URL to access.  If that is unspecified, it will fallback to
> -# the hardcoded default value specified here.
> -DEFAULT_URL = "http://patchwork/xmlrpc/"
> -CONFIG_FILE = os.path.expanduser('~/.pwclientrc')
> -
> -
> -class Filter(object):
> -
> -    """Filter for selecting patches."""
> -
> -    def __init__(self):
> -        # These fields refer to specific objects, so they are special
> -        # because we have to resolve them to IDs before passing the
> -        # filter to the server
> -        self.state = ""
> -        self.project = ""
> -
> -        # The dictionary that gets passed to via XML-RPC
> -        self.d = {}
> -
> -    def add(self, field, value):
> -        if field == 'state':
> -            self.state = value
> -        elif field == 'project':
> -            self.project = value
> -        else:
> -            # OK to add directly
> -            self.d[field] = value
> -
> -    def resolve_ids(self, rpc):
> -        """Resolve State, Project, and Person IDs based on filter strings."""
> -        if self.state != "":
> -            id = state_id_by_name(rpc, self.state)
> -            if id == 0:
> -                sys.stderr.write("Note: No State found matching %s*, "
> -                                 "ignoring filter\n" % self.state)
> -            else:
> -                self.d['state_id'] = id
> -
> -        if self.project is not None:
> -            id = project_id_by_name(rpc, self.project)
> -            if id == 0:
> -                sys.stderr.write("Note: No Project found matching %s, "
> -                                 "ignoring filter\n" % self.project)
> -            else:
> -                self.d['project_id'] = id
> -
> -    def __str__(self):
> -        """Return human-readable description of the filter."""
> -        return str(self.d)
> -
> -
> -if sys.version_info[0] < 3:
> -    # the python 2.7 reference implementation tries to re-encode to
> -    # ascii bytes here but leaves unicode if it fails. Do not try to
> -    # re-encode to ascii byte string to have a more predictive behavior.
> -    xmlrpclib._stringify = lambda s: s
> -
> -
> -class Transport(xmlrpclib.SafeTransport):
> -
> -    def __init__(self, url):
> -        xmlrpclib.SafeTransport.__init__(self)
> -        self.credentials = None
> -        self.host = None
> -        self.proxy = None
> -        self.scheme = url.split('://', 1)[0]
> -        self.https = url.startswith('https')
> -        if self.https:
> -            self.proxy = os.environ.get('https_proxy')
> -        else:
> -            self.proxy = os.environ.get('http_proxy')
> -        if self.proxy:
> -            self.https = self.proxy.startswith('https')
> -
> -    def set_credentials(self, username=None, password=None):
> -        self.credentials = '%s:%s' % (username, password)
> -
> -    def make_connection(self, host):
> -        self.host = host
> -        if self.proxy:
> -            host = self.proxy.split('://', 1)[-1].rstrip('/')
> -        if self.credentials:
> -            host = '@'.join([self.credentials, host])
> -        if self.https:
> -            return xmlrpclib.SafeTransport.make_connection(self, host)
> -        else:
> -            return xmlrpclib.Transport.make_connection(self, host)
> -
> -    if sys.version_info[0] == 2:
> -        def send_request(self, connection, handler, request_body):
> -            handler = '%s://%s%s' % (self.scheme, self.host, handler)
> -            xmlrpclib.Transport.send_request(self, connection, handler,
> -                                             request_body)
> -    else:  # Python 3
> -        def send_request(self, host, handler, request_body, debug):
> -            handler = '%s://%s%s' % (self.scheme, host, handler)
> -            return xmlrpclib.Transport.send_request(self, host, handler,
> -                                                    request_body, debug)
> -
> -
> -def project_id_by_name(rpc, linkname):
> -    """Given a project short name, look up the Project ID."""
> -    if len(linkname) == 0:
> -        return 0
> -    projects = rpc.project_list(linkname, 0)
> -    for project in projects:
> -        if project['linkname'] == linkname:
> -            return project['id']
> -    return 0
> -
> -
> -def state_id_by_name(rpc, name):
> -    """Given a partial state name, look up the state ID."""
> -    if len(name) == 0:
> -        return 0
> -    states = rpc.state_list(name, 0)
> -    for state in states:
> -        if state['name'].lower().startswith(name.lower()):
> -            return state['id']
> -    return 0
> -
> -
> -def person_ids_by_name(rpc, name):
> -    """Given a partial name or email address, return a list of the
> -    person IDs that match."""
> -    if len(name) == 0:
> -        return []
> -    people = rpc.person_list(name, 0)
> -    return [x['id'] for x in people]
> -
> -
> -def list_patches(patches, format_str=None):
> -    """Dump a list of patches to stdout."""
> -    if format_str:
> -        format_field_re = re.compile("%{([a-z0-9_]+)}")
> -
> -        def patch_field(matchobj):
> -            fieldname = matchobj.group(1)
> -
> -            if fieldname == "_msgid_":
> -                # naive way to strip < and > from message-id
> -                val = string.strip(str(patch["msgid"]), "<>")
> -            else:
> -                val = str(patch[fieldname])
> -
> -            return val
> -
> -        for patch in patches:
> -            print(format_field_re.sub(patch_field, format_str))
> -    else:
> -        print("%-7s %-12s %s" % ("ID", "State", "Name"))
> -        print("%-7s %-12s %s" % ("--", "-----", "----"))
> -        for patch in patches:
> -            print("%-7d %-12s %s" %
> -                  (patch['id'], patch['state'], patch['name']))
> -
> -
> -def action_list(rpc, filter, submitter_str, delegate_str, format_str=None):
> -    filter.resolve_ids(rpc)
> -
> -    if submitter_str is not None:
> -        ids = person_ids_by_name(rpc, submitter_str)
> -        if len(ids) == 0:
> -            sys.stderr.write("Note: Nobody found matching *%s*\n" %
> -                             submitter_str)
> -        else:
> -            for id in ids:
> -                person = rpc.person_get(id)
> -                print('Patches submitted by %s <%s>:' %
> -                      (person['name'], person['email']))
> -                f = filter
> -                f.add("submitter_id", id)
> -                patches = rpc.patch_list(f.d)
> -                list_patches(patches, format_str)
> -        return
> -
> -    if delegate_str is not None:
> -        ids = person_ids_by_name(rpc, delegate_str)
> -        if len(ids) == 0:
> -            sys.stderr.write("Note: Nobody found matching *%s*\n" %
> -                             delegate_str)
> -        else:
> -            for id in ids:
> -                person = rpc.person_get(id)
> -                print('Patches delegated to %s <%s>:' %
> -                      (person['name'], person['email']))
> -                f = filter
> -                f.add("delegate_id", id)
> -                patches = rpc.patch_list(f.d)
> -                list_patches(patches, format_str)
> -        return
> -
> -    patches = rpc.patch_list(filter.d)
> -    list_patches(patches, format_str)
> -
> -
> -def action_projects(rpc):
> -    projects = rpc.project_list("", 0)
> -    print("%-5s %-24s %s" % ("ID", "Name", "Description"))
> -    print("%-5s %-24s %s" % ("--", "----", "-----------"))
> -    for project in projects:
> -        print("%-5d %-24s %s" % (project['id'],
> -                                 project['linkname'],
> -                                 project['name']))
> -
> -
> -def action_check_list(rpc):
> -    checks = rpc.check_list()
> -    print("%-5s %-16s %-8s %s" % ("ID", "Context", "State", "Patch"))
> -    print("%-5s %-16s %-8s %s" % ("--", "-------", "-----", "-----"))
> -    for check in checks:
> -        print("%-5s %-16s %-8s %s" % (check['id'],
> -                                      check['context'],
> -                                      check['state'],
> -                                      check['patch']))
> -
> -
> -def action_check_info(rpc, check_id):
> -    check = rpc.check_get(check_id)
> -    s = "Information for check id %d" % (check_id)
> -    print(s)
> -    print('-' * len(s))
> -    for key, value in sorted(check.items()):
> -        print("- %- 14s: %s" % (key, value))
> -
> -
> -def action_check_create(rpc, patch_id, context, state, url, description):
> -    try:
> -        rpc.check_create(patch_id, context, state, url, description)
> -    except xmlrpclib.Fault as f:
> -        sys.stderr.write("Error creating check: %s\n" % f.faultString)
> -
> -
> -def action_states(rpc):
> -    states = rpc.state_list("", 0)
> -    print("%-5s %s" % ("ID", "Name"))
> -    print("%-5s %s" % ("--", "----"))
> -    for state in states:
> -        print("%-5d %s" % (state['id'], state['name']))
> -
> -
> -def action_info(rpc, patch_id):
> -    patch = rpc.patch_get(patch_id)
> -    s = "Information for patch id %d" % (patch_id)
> -    print(s)
> -    print('-' * len(s))
> -    for key, value in sorted(patch.items()):
> -        print("- %- 14s: %s" % (key, value))
> -
> -
> -def action_get(rpc, patch_id):
> -    patch = rpc.patch_get(patch_id)
> -    s = rpc.patch_get_mbox(patch_id)
> -
> -    if patch == {} or len(s) == 0:
> -        sys.stderr.write("Unable to get patch %d\n" % patch_id)
> -        sys.exit(1)
> -
> -    base_fname = fname = os.path.basename(patch['filename'])
> -    fname += '.patch'
> -    i = 0
> -    while os.path.exists(fname):
> -        fname = "%s.%d.patch" % (base_fname, i)
> -        i += 1
> -
> -    with io.open(fname, 'w', encoding='utf-8') as f:
> -        f.write(s)
> -        print('Saved patch to %s' % fname)
> -
> -
> -def action_apply(rpc, patch_id, apply_cmd=None):
> -    patch = rpc.patch_get(patch_id)
> -    if patch == {}:
> -        sys.stderr.write("Error getting information on patch ID %d\n" %
> -                         patch_id)
> -        sys.exit(1)
> -
> -    if apply_cmd is None:
> -        print('Applying patch #%d to current directory' % patch_id)
> -        apply_cmd = ['patch', '-p1']
> -    else:
> -        print('Applying patch #%d using "%s"' %
> -              (patch_id, ' '.join(apply_cmd)))
> -
> -    print('Description: %s' % patch['name'])
> -    s = rpc.patch_get_mbox(patch_id)
> -    if len(s) > 0:
> -        proc = subprocess.Popen(apply_cmd, stdin=subprocess.PIPE)
> -        proc.communicate(s.encode('utf-8'))
> -        return proc.returncode
> -    else:
> -        sys.stderr.write("Error: No patch content found\n")
> -        sys.exit(1)
> -
> -
> -def action_update_patch(rpc, patch_id, state=None, archived=None, commit=None):
> -    patch = rpc.patch_get(patch_id)
> -    if patch == {}:
> -        sys.stderr.write("Error getting information on patch ID %d\n" %
> -                         patch_id)
> -        sys.exit(1)
> -
> -    params = {}
> -
> -    if state:
> -        state_id = state_id_by_name(rpc, state)
> -        if state_id == 0:
> -            sys.stderr.write("Error: No State found matching %s*\n" % state)
> -            sys.exit(1)
> -        params['state'] = state_id
> -
> -    if commit:
> -        params['commit_ref'] = commit
> -
> -    if archived:
> -        params['archived'] = archived == 'yes'
> -
> -    success = False
> -    try:
> -        success = rpc.patch_set(patch_id, params)
> -    except xmlrpclib.Fault as f:
> -        sys.stderr.write("Error updating patch: %s\n" % f.faultString)
> -
> -    if not success:
> -        sys.stderr.write("Patch not updated\n")
> -
> -
> -def patch_id_from_hash(rpc, project, hash):
> -    try:
> -        patch = rpc.patch_get_by_project_hash(project, hash)
> -    except xmlrpclib.Fault:
> -        # the server may not have the newer patch_get_by_project_hash function,
> -        # so fall back to hash-only.
> -        patch = rpc.patch_get_by_hash(hash)
> -
> -    if patch == {}:
> -        sys.stderr.write("No patch has the hash provided\n")
> -        sys.exit(1)
> -
> -    patch_id = patch['id']
> -    # be super paranoid
> -    try:
> -        patch_id = int(patch_id)
> -    except ValueError:
> -        sys.stderr.write("Invalid patch ID obtained from server\n")
> -        sys.exit(1)
> -    return patch_id
> -
> -
> -auth_actions = ['check_create', 'update']
> -
> -
> -def main():
> -    hash_parser = argparse.ArgumentParser(add_help=False)
> -    hash_parser.add_argument(
> -        '-h', metavar='HASH', dest='hash', action='store',
> -        help='''Lookup by patch hash'''
> -    )
> -    hash_parser.add_argument(
> -        'id', metavar='ID', nargs='*', action='store', type=int,
> -        help='Patch ID',
> -    )
> -    hash_parser.add_argument(
> -        '-p', metavar='PROJECT',
> -        help='''Lookup patch in project'''
> -    )
> -
> -    filter_parser = argparse.ArgumentParser(add_help=False)
> -    filter_parser.add_argument(
> -        '-s', metavar='STATE',
> -        help='''Filter by patch state (e.g., 'New', 'Accepted', etc.)'''
> -    )
> -    filter_parser.add_argument(
> -        '-a', choices=['yes', 'no'],
> -        help='''Filter by patch archived state'''
> -    )
> -    filter_parser.add_argument(
> -        '-p', metavar='PROJECT',
> -        help='''Filter by project name (see 'projects' for list)'''
> -    )
> -    filter_parser.add_argument(
> -        '-w', metavar='WHO',
> -        help='''Filter by submitter (name, e-mail substring search)'''
> -    )
> -    filter_parser.add_argument(
> -        '-d', metavar='WHO',
> -        help='''Filter by delegate (name, e-mail substring search)'''
> -    )
> -    filter_parser.add_argument(
> -        '-n', metavar='MAX#', type=int,
> -        help='''Return first n results'''
> -    )
> -    filter_parser.add_argument(
> -        '-N', metavar='MAX#', type=int,
> -        help='''Return last N results'''
> -    )
> -    filter_parser.add_argument(
> -        '-m', metavar='MESSAGEID',
> -        help='''Filter by Message-Id'''
> -    )
> -    filter_parser.add_argument(
> -        '-f', metavar='FORMAT',
> -        help='''Print output in the given format. You can use tags matching '''
> -        '''fields, e.g. %%{id}, %%{state}, or %%{msgid}.'''
> -    )
> -    filter_parser.add_argument(
> -        'patch_name', metavar='STR', nargs='?',
> -        help='substring to search for patches by name',
> -    )
> -
> -    action_parser = argparse.ArgumentParser(
> -        prog='pwclient',
> -        formatter_class=argparse.RawTextHelpFormatter,
> -        epilog="""Use 'pwclient <command> --help' for more info.
> -
> -To avoid unicode encode/decode errors, you should export the LANG or LC_ALL
> -environment variables according to the configured locales on your system. If
> -these variables are already set, make sure that they point to valid and
> -installed locales.
> -""",
> -    )
> -
> -    subparsers = action_parser.add_subparsers(
> -        title='Commands',
> -    )
> -    apply_parser = subparsers.add_parser(
> -        'apply', parents=[hash_parser], conflict_handler='resolve',
> -        help='''Apply a patch (in the current dir, using -p1)'''
> -    )
> -    apply_parser.set_defaults(subcmd='apply')
> -    git_am_parser = subparsers.add_parser(
> -        'git-am', parents=[hash_parser], conflict_handler='resolve',
> -        help='''Apply a patch to current git branch using "git am".'''
> -    )
> -    git_am_parser.set_defaults(subcmd='git_am')
> -    git_am_parser.add_argument(
> -        '-s', '--signoff',
> -        action='store_true',
> -        help='''pass --signoff to git-am'''
> -    )
> -    git_am_parser.add_argument(
> -        '-3', '--3way',
> -        action='store_true',
> -        help='''pass --3way to git-am'''
> -    )
> -    get_parser = subparsers.add_parser(
> -        'get', parents=[hash_parser], conflict_handler='resolve',
> -        help='''Download a patch and save it locally'''
> -    )
> -    get_parser.set_defaults(subcmd='get')
> -    info_parser = subparsers.add_parser(
> -        'info', parents=[hash_parser], conflict_handler='resolve',
> -        help='''Display patchwork info about a given patch ID'''
> -    )
> -    info_parser.set_defaults(subcmd='info')
> -    projects_parser = subparsers.add_parser(
> -        'projects',
> -        help='''List all projects'''
> -    )
> -    projects_parser.set_defaults(subcmd='projects')
> -    check_list_parser = subparsers.add_parser(
> -        'check-list',
> -        add_help=False,
> -        help='''List all checks'''
> -    )
> -    check_list_parser.set_defaults(subcmd='check_list')
> -    check_info_parser = subparsers.add_parser(
> -        'check-info',
> -        add_help=False,
> -        help='''Show information for a given check'''
> -    )
> -    check_info_parser.set_defaults(subcmd='check_info')
> -    check_info_parser.add_argument(
> -        'check_id', metavar='ID', action='store', type=int,
> -        help='Check ID',)
> -    check_create_parser = subparsers.add_parser(
> -        'check-create', parents=[hash_parser], conflict_handler='resolve',
> -        help='Add a check to a patch')
> -    check_create_parser.set_defaults(subcmd='check_create')
> -    check_create_parser.add_argument(
> -        '-c', metavar='CONTEXT')
> -    check_create_parser.add_argument(
> -        '-s', choices=('pending', 'success', 'warning', 'fail'))
> -    check_create_parser.add_argument(
> -        '-u', metavar='TARGET_URL', default="")
> -    check_create_parser.add_argument(
> -        '-d', metavar='DESCRIPTION', default="")
> -    states_parser = subparsers.add_parser(
> -        'states',
> -        help='''Show list of potential patch states'''
> -    )
> -    states_parser.set_defaults(subcmd='states')
> -    view_parser = subparsers.add_parser(
> -        'view', parents=[hash_parser], conflict_handler='resolve',
> -        help='''View a patch'''
> -    )
> -    view_parser.set_defaults(subcmd='view')
> -    update_parser = subparsers.add_parser(
> -        'update', parents=[hash_parser], conflict_handler='resolve',
> -        help='''Update patch''',
> -        epilog='''Using a COMMIT-REF allows for only one ID to be specified''',
> -    )
> -    update_parser.add_argument(
> -        '-c', metavar='COMMIT-REF',
> -        help='''commit reference hash'''
> -    )
> -    update_parser.add_argument(
> -        '-s', metavar='STATE',
> -        help='''Set patch state (e.g., 'Accepted', 'Superseded' etc.)'''
> -    )
> -    update_parser.add_argument(
> -        '-a', choices=['yes', 'no'],
> -        help='''Set patch archived state'''
> -    )
> -    update_parser.set_defaults(subcmd='update')
> -    list_parser = subparsers.add_parser(
> -        'list', parents=[filter_parser],
> -        help='List patches using optional filters')
> -    list_parser.set_defaults(subcmd='list')
> -    search_parser = subparsers.add_parser("search",
> -                                          parents=[filter_parser],
> -                                          help='''Alias for "list"'''
> -                                          )
> -    # Poor man's argparse aliases:
> -    # We register the "search" parser but effectively use "list" for the
> -    # help-text.
> -    search_parser.set_defaults(subcmd='list')
> -    if len(sys.argv) < 2:
> -        action_parser.print_help()
> -        sys.exit(0)
> -
> -    args = action_parser.parse_args()
> -    args = dict(vars(args))
> -    action = args.get('subcmd')
> -
> -    if args.get('hash') and len(args.get('id')):
> -        # mimic mutual exclusive group
> -        locals()[action + '_parser'].error(
> -            "[-h HASH] and [ID [ID ...]] are mutually exclusive")
> -
> -    # set defaults
> -    filt = Filter()
> -    commit_str = None
> -    url = DEFAULT_URL
> -
> -    archived_str = args.get('a')
> -    state_str = args.get('s')
> -    project_str = args.get('p')
> -    submitter_str = args.get('w')
> -    delegate_str = args.get('d')
> -    format_str = args.get('f')
> -    hash_str = args.get('hash')
> -    patch_ids = args.get('id')
> -    msgid_str = args.get('m')
> -    if args.get('c'):
> -        # update multiple IDs with a single commit-hash does not make sense
> -        if action == 'update' and patch_ids and len(patch_ids) > 1:
> -            update_parser.error(
> -                "Declining update with COMMIT-REF on multiple IDs")
> -        commit_str = args.get('c')
> -
> -    if state_str is None and archived_str is None and action == 'update':
> -        update_parser.error(
> -            'Must specify one or more update options (-a or -s)')
> -
> -    if args.get('n') is not None:
> -        filt.add("max_count", args.get('n'))
> -
> -    if args.get('N') is not None:
> -        filt.add("max_count", 0 - args.get('N'))
> -
> -    do_signoff = args.get('signoff')
> -    do_three_way = args.get('3way')
> -
> -    # grab settings from config files
> -    config = ConfigParser.ConfigParser()
> -    config.read([CONFIG_FILE])
> -
> -    if not config.has_section('options') and os.path.exists(CONFIG_FILE):
> -        sys.stderr.write('%s is in the old format. Migrating it...' %
> -                         CONFIG_FILE)
> -
> -        old_project = config.get('base', 'project')
> -
> -        new_config = ConfigParser.ConfigParser()
> -        new_config.add_section('options')
> -
> -        new_config.set('options', 'default', old_project)
> -        new_config.add_section(old_project)
> -
> -        new_config.set(old_project, 'url', config.get('base', 'url'))
> -        if config.has_option('auth', 'username'):
> -            new_config.set(
> -                old_project, 'username', config.get('auth', 'username'))
> -        if config.has_option('auth', 'password'):
> -            new_config.set(
> -                old_project, 'password', config.get('auth', 'password'))
> -
> -        old_config_file = CONFIG_FILE + '.orig'
> -        shutil.copy2(CONFIG_FILE, old_config_file)
> -
> -        with open(CONFIG_FILE, 'wb') as fd:
> -            new_config.write(fd)
> -
> -        sys.stderr.write(' Done.\n')
> -        sys.stderr.write(
> -            'Your old %s was saved to %s\n' % (CONFIG_FILE, old_config_file))
> -        sys.stderr.write(
> -            'and was converted to the new format. You may want to\n')
> -        sys.stderr.write('inspect it before continuing.\n')
> -        sys.exit(1)
> -
> -    if not project_str:
> -        try:
> -            project_str = config.get('options', 'default')
> -        except (ConfigParser.NoSectionError, ConfigParser.NoOptionError):
> -            action_parser.error(
> -                "No default project configured in %s\n" % CONFIG_FILE)
> -
> -    if not config.has_section(project_str):
> -        sys.stderr.write(
> -            'No section for project %s in %s\n' % (CONFIG_FILE, project_str))
> -        sys.exit(1)
> -    if not config.has_option(project_str, 'url'):
> -        sys.stderr.write(
> -            'No URL for project %s in %s\n' % (CONFIG_FILE, project_str))
> -        sys.exit(1)
> -    if not do_signoff and config.has_option('options', 'signoff'):
> -        do_signoff = config.getboolean('options', 'signoff')
> -    if not do_signoff and config.has_option(project_str, 'signoff'):
> -        do_signoff = config.getboolean(project_str, 'signoff')
> -    if not do_three_way and config.has_option('options', '3way'):
> -        do_three_way = config.getboolean('options', '3way')
> -    if not do_three_way and config.has_option(project_str, '3way'):
> -        do_three_way = config.getboolean(project_str, '3way')
> -
> -    url = config.get(project_str, 'url')
> -
> -    transport = Transport(url)
> -    if action in auth_actions:
> -        if config.has_option(project_str, 'username') and \
> -                config.has_option(project_str, 'password'):
> -            transport.set_credentials(
> -                config.get(project_str, 'username'),
> -                config.get(project_str, 'password'))
> -        else:
> -            sys.stderr.write("The %s action requires authentication, but no "
> -                             "username or password\nis configured\n" % action)
> -            sys.exit(1)
> -
> -    if project_str:
> -        filt.add("project", project_str)
> -
> -    if state_str:
> -        filt.add("state", state_str)
> -
> -    if archived_str:
> -        filt.add("archived", archived_str == 'yes')
> -
> -    if msgid_str:
> -        filt.add("msgid", msgid_str)
> -
> -    try:
> -        rpc = xmlrpclib.Server(url, transport=transport)
> -    except (IOError, OSError):
> -        sys.stderr.write("Unable to connect to %s\n" % url)
> -        sys.exit(1)
> -
> -    # It should be safe to assume hash_str is not zero, but who knows..
> -    if hash_str is not None:
> -        patch_ids = [patch_id_from_hash(rpc, project_str, hash_str)]
> -
> -    # helper for non_empty() to print correct helptext
> -    h = locals()[action + '_parser']
> -
> -    # Require either hash_str or IDs for
> -    def non_empty(h, patch_ids):
> -        """Error out if no patch IDs were specified"""
> -        if patch_ids is None or len(patch_ids) < 1:
> -            sys.stderr.write("Error: Missing Argument! Either [-h HASH] or "
> -                             "[ID [ID ...]] are required\n")
> -            if h:
> -                h.print_help()
> -            sys.exit(1)
> -        return patch_ids
> -
> -    if action == 'list' or action == 'search':
> -        if args.get('patch_name') is not None:
> -            filt.add("name__icontains", args.get('patch_name'))
> -        action_list(rpc, filt, submitter_str, delegate_str, format_str)
> -
> -    elif action.startswith('project'):
> -        action_projects(rpc)
> -
> -    elif action.startswith('state'):
> -        action_states(rpc)
> -
> -    elif action == 'view':
> -        pager = os.environ.get('PAGER')
> -        if pager:
> -            pager = subprocess.Popen(
> -                pager.split(), stdin=subprocess.PIPE
> -            )
> -        if pager:
> -            i = list()
> -            for patch_id in non_empty(h, patch_ids):
> -                s = rpc.patch_get_mbox(patch_id)
> -                if len(s) > 0:
> -                    i.append(s)
> -            if len(i) > 0:
> -                pager.communicate(input="\n".join(i).encode("utf-8"))
> -            pager.stdin.close()
> -        else:
> -            for patch_id in non_empty(h, patch_ids):
> -                s = rpc.patch_get_mbox(patch_id)
> -                if len(s) > 0:
> -                    print(s)
> -
> -    elif action == 'info':
> -        for patch_id in non_empty(h, patch_ids):
> -            action_info(rpc, patch_id)
> -
> -    elif action == 'get':
> -        for patch_id in non_empty(h, patch_ids):
> -            action_get(rpc, patch_id)
> -
> -    elif action == 'apply':
> -        for patch_id in non_empty(h, patch_ids):
> -            ret = action_apply(rpc, patch_id)
> -            if ret:
> -                sys.stderr.write("Apply failed with exit status %d\n" % ret)
> -                sys.exit(1)
> -
> -    elif action == 'git_am':
> -        cmd = ['git', 'am']
> -        if do_signoff:
> -            cmd.append('-s')
> -        if do_three_way:
> -            cmd.append('-3')
> -        for patch_id in non_empty(h, patch_ids):
> -            ret = action_apply(rpc, patch_id, cmd)
> -            if ret:
> -                sys.stderr.write("'git am' failed with exit status %d\n" % ret)
> -                sys.exit(1)
> -
> -    elif action == 'update':
> -        for patch_id in non_empty(h, patch_ids):
> -            action_update_patch(rpc, patch_id, state=state_str,
> -                                archived=archived_str, commit=commit_str
> -                                )
> -
> -    elif action == 'check_list':
> -        action_check_list(rpc)
> -
> -    elif action == 'check_info':
> -        check_id = args['check_id']
> -        action_check_info(rpc, check_id)
> -
> -    elif action == 'check_create':
> -        for patch_id in non_empty(h, patch_ids):
> -            action_check_create(
> -                rpc, patch_id, args['c'], args['s'], args['u'], args['d'])
> -
> -    else:
> -        sys.stderr.write("Unknown action '%s'\n" % action)
> -        action_parser.print_help()
> -        sys.exit(1)
> -
> -
> -if __name__ == "__main__":
> -    try:
> -        main()
> -    except (UnicodeEncodeError, UnicodeDecodeError) as e:
> -        import traceback
> -        traceback.print_exc()
> -        sys.stderr.write('Try exporting the LANG or LC_ALL env vars. See '
> -                         'pwclient --help for more details.\n')
> -        sys.exit(1)
> diff --git a/patchwork/templates/patchwork/project.html b/patchwork/templates/patchwork/project.html
> index 74b6f0fb..99e36ff7 100644
> --- a/patchwork/templates/patchwork/project.html
> +++ b/patchwork/templates/patchwork/project.html
> @@ -58,8 +58,7 @@ and applying patches.</p>
>  
>  <p>To use pwclient, you will need:</p>
>  <ul>
> - <li>The <a href="{% url 'pwclient' %}">pwclient</a>
> -  program (11kB, python script)</li>
> + <li>The <a href="https://github.com/getpatchwork/pwclient">pwclient</a> program</li>
>   <li>(optional) A <code><a href="{% url 'pwclientrc' project.linkname %}"
>   >.pwclientrc</a></code> file for this project, which should be stored in your
>   home directory.</li>
> diff --git a/patchwork/urls.py b/patchwork/urls.py
> index 935e25fa..cfcf04dc 100644
> --- a/patchwork/urls.py
> +++ b/patchwork/urls.py
> @@ -143,8 +143,6 @@ if 'debug_toolbar' in settings.INSTALLED_APPS:
>  if settings.ENABLE_XMLRPC:
>      urlpatterns += [
>          url(r'xmlrpc/$', xmlrpc_views.xmlrpc, name='xmlrpc'),
> -        url(r'^pwclient/$', pwclient_views.pwclient,
> -            name='pwclient'),
>          url(r'^project/(?P<project_id>[^/]+)/pwclientrc/$',
>              pwclient_views.pwclientrc,
>              name='pwclientrc'),
> diff --git a/patchwork/views/pwclient.py b/patchwork/views/pwclient.py
> index c6d8b342..72ebcbbb 100644
> --- a/patchwork/views/pwclient.py
> +++ b/patchwork/views/pwclient.py
> @@ -26,11 +26,3 @@ def pwclientrc(request, project_id):
>      response['Content-Disposition'] = 'attachment; filename=.pwclientrc'
>  
>      return response
> -
> -
> -def pwclient(request):
> -    response = render(request, 'patchwork/pwclient',
> -                      content_type='text/x-python')
> -    response['Content-Disposition'] = 'attachment; filename=pwclient'
> -
> -    return response
> diff --git a/releasenotes/notes/remove-pwclient-2ad030cdc1425e80.yaml b/releasenotes/notes/remove-pwclient-2ad030cdc1425e80.yaml
> new file mode 100644
> index 00000000..6c81f807
> --- /dev/null
> +++ b/releasenotes/notes/remove-pwclient-2ad030cdc1425e80.yaml
> @@ -0,0 +1,8 @@
> +---
> +upgrade:
> +  - |
> +    ``pwclient`` is no longer packaged with Patchwork. Instead, it is developed
> +    as a separate project on `GitHub`__ and available from `PyPI`__.
> +
> +    __ https://github.com/getpatchwork/pwclient
> +    __ https://pypi.org/project/pwclient/
> diff --git a/tools/patchwork-update-commits b/tools/patchwork-update-commits
> index 62c91c32..269dac9e 100755
> --- a/tools/patchwork-update-commits
> +++ b/tools/patchwork-update-commits
> @@ -16,5 +16,5 @@ fi
>  git rev-list --reverse "$@" |
>  while read -r commit; do
>      hash=$(git diff "$commit~..$commit" | python "$PW_DIR/hasher.py")
> -    "$PW_DIR/bin/pwclient" update -s Accepted -c "$commit" -h "$hash"
> +    pwclient update -s Accepted -c "$commit" -h "$hash"
>  done
> diff --git a/tools/post-receive.hook b/tools/post-receive.hook
> index 81a519ef..9f2f0503 100755
> --- a/tools/post-receive.hook
> +++ b/tools/post-receive.hook
> @@ -30,14 +30,14 @@ get_patchwork_hash() {
>  
>  get_patch_id() {
>      local id
> -    id=$($PW_DIR/bin/pwclient info -h "$1" 2>/dev/null \
> -         | sed -rne 's,- id[[:space:]]*: ,,p')
> +    id=$(pwclient info -h "$1" 2>/dev/null | \
> +         sed -rne 's,- id[[:space:]]*: ,,p')
>      echo "$id"
>      test -n "$id"
>  }
>  
>  set_patch_state() {
> -    $PW_DIR/bin/pwclient update -s "$2" -c "$3" "$1" 2>&1
> +    pwclient update -s "$2" -c "$3" "$1" 2>&1
>  }
>  
>  update_patches() {
> diff --git a/tox.ini b/tox.ini
> index 384d3c7c..c98f755e 100644
> --- a/tox.ini
> +++ b/tox.ini
> @@ -40,7 +40,7 @@ commands =
>  [testenv:pep8]
>  basepython = python2.7
>  deps = flake8
> -commands = flake8 {posargs} patchwork patchwork/bin/pwclient
> +commands = flake8 {posargs} patchwork
>  
>  [flake8]
>  ignore = E129, F405
> -- 
> 2.17.2
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

On Tue, 2018-10-23 at 01:31 +1100, Daniel Axtens wrote:
> Stephen Finucane <stephen@that.guru> writes:
> 
> > Let's start managing this via a separate project, which will allow the
> > client to evolve separately from the server. No redirect is added for
> > the old '/pwclient' URL as it seems wiser to return a HTTP 404 error
> > code.

So...

> I can't say I'm thrilled by the idea. It adds complexity and I don't
> know what it would make easier. Presumably patches for it would still go
> to this list and be reviewed in the usual way, so I'm not sure how it
> would help workflow.

Yup, I plan on keeping development on the same mailing list, if at all
possible (more on this in a bit).

> It's also a single file at the moment, so it's not like it's so
> complex that it should be spun out to simplify things...

But this is changing. There are two things I'd like to do here that
make shipping this separately seem like a a smarter thing.

 * I want to start shipping pwclient on PyPI so I can just 'pip
   install' wherever I need it (and maybe 'dnf install', in the
   future).

 * I want to add REST API support but would rather not duplicate the
   underlying client in git-pw and pwclient (and whatever other clients
   may be developed in the future). To this end, I'd like to develop a
   'pwlib' library that will provide a generic API client, meaning
   'pwclient' and 'git-pw' will simply provide the user chrome.

These two points are related. Assuming we ship this on PyPI, we're
going to need to add some of distribution tooling (pbr, in this case,
though setuptools or plain old distutils would also be valid options).
I'll also distribution tooling if I want to follow through on 'pwlib'
plan, both to allow 'pwlib' itself to be distributed via PyPI and so
'pwclient' can pull this in as part of it's installation (I really
don't want to vendor pwlib in-tree). All of this means copy-pasting the
script will no longer be an option and the single file we currently
have will grow into multiple files before long. While could keep
developing pwclient (and optionally pwlib) in-tree, doing so makes life
harder for us with regard to things like versioning, documentation,
distribution, etc. Similarly, while we could get away with it for
single file script, it seems wrong to package multiple complex projects
within the same tree, each with their own versioning, documentation,
and release processes. The server will always take priority, meaning
things like tags, the 'docs' directory, 'tox' files etc. will apply
first and foremost to that project.

'git-pw' is already its own project and I think this has allowed it to
develop faster than it might have done were it found in tree. The only
thing I haven't done with this is develop it fully on the list (mostly
because I think of it as a third-party sort of tool) but we can
definitely continue to develop pwclient on list. In fact, the Patchwork
2.1 'project.subject_match' feature allows us to this (and gives us a
way to dog food the feature).

> Do you have a plan to avoid losing the git history if you do this move?

Yup, I've already pulled this out (strictly as a test, until we make a
decision here). The history is retained [1] and I've documented the
steps in the repo itself [2]. I've also pushed up some of the future
changes I have prepared [3], which see us breaking up this monolithic
file into smaller chunks, in preparation for the eventual addition of
REST API support along with some smaller fixes.

> I am somewhat biased because I almost always just invoke pwclient from
> the project directory as patchwork/bin/pwclient, so this would totally
> break my usecase, but I realise that's not a valid reason per se.

Stephen

> Regards,
> Daniel

PS: As an aside, I do think this shows an interesting difference in our
backgrounds shows up :) I'm used to working with multiple different
repos in OpenStack, where everything is split out into smaller units
and versioned APIs ensure different components talk to each other. As
such, splitting this out feels natural to me. However, I guess the
kernel, with it's monolithic repo, does things differently and I'm
guessing tooling for that is all kept in-tree? Interesting, in any
case.

[1] https://github.com/getpatchwork/pwclient/commits/master
[2] https://github.com/getpatchwork/pwclient/commit/23fd64ad3
[1] https://github.com/getpatchwork/pwclient/commits/devel

On Mon, 2018-10-22 at 16:55 +0100, Stephen Finucane wrote:
> On Tue, 2018-10-23 at 01:31 +1100, Daniel Axtens wrote:
> > Stephen Finucane <stephen@that.guru> writes:
> > 
> > > Let's start managing this via a separate project, which will allow the
> > > client to evolve separately from the server. No redirect is added for
> > > the old '/pwclient' URL as it seems wiser to return a HTTP 404 error
> > > code.
> 
> So...
> 
> > I can't say I'm thrilled by the idea. It adds complexity and I don't
> > know what it would make easier. Presumably patches for it would still go
> > to this list and be reviewed in the usual way, so I'm not sure how it
> > would help workflow.
> 
> Yup, I plan on keeping development on the same mailing list, if at all
> possible (more on this in a bit).
> 
> > It's also a single file at the moment, so it's not like it's so
> > complex that it should be spun out to simplify things...
> 
> But this is changing. There are two things I'd like to do here that
> make shipping this separately seem like a a smarter thing.
> 
>  * I want to start shipping pwclient on PyPI so I can just 'pip
>    install' wherever I need it (and maybe 'dnf install', in the
>    future).
> 
>  * I want to add REST API support but would rather not duplicate the
>    underlying client in git-pw and pwclient (and whatever other clients
>    may be developed in the future). To this end, I'd like to develop a
>    'pwlib' library that will provide a generic API client, meaning
>    'pwclient' and 'git-pw' will simply provide the user chrome.
> 
> These two points are related. Assuming we ship this on PyPI, we're
> going to need to add some of distribution tooling (pbr, in this case,
> though setuptools or plain old distutils would also be valid options).
> I'll also distribution tooling if I want to follow through on 'pwlib'
> plan, both to allow 'pwlib' itself to be distributed via PyPI and so
> 'pwclient' can pull this in as part of it's installation (I really
> don't want to vendor pwlib in-tree). All of this means copy-pasting the
> script will no longer be an option and the single file we currently
> have will grow into multiple files before long. While could keep
> developing pwclient (and optionally pwlib) in-tree, doing so makes life
> harder for us with regard to things like versioning, documentation,
> distribution, etc. Similarly, while we could get away with it for
> single file script, it seems wrong to package multiple complex projects
> within the same tree, each with their own versioning, documentation,
> and release processes. The server will always take priority, meaning
> things like tags, the 'docs' directory, 'tox' files etc. will apply
> first and foremost to that project.
> 
> 'git-pw' is already its own project and I think this has allowed it to
> develop faster than it might have done were it found in tree. The only
> thing I haven't done with this is develop it fully on the list (mostly
> because I think of it as a third-party sort of tool) but we can
> definitely continue to develop pwclient on list. In fact, the Patchwork
> 2.1 'project.subject_match' feature allows us to this (and gives us a
> way to dog food the feature).

Now that the ozlabs instance has been updated to v2.1, we have access
to the subject_match feature and so I'd like to proceed with this
migration. The 'patchwork/pwclient' repo is seeded and I have a large
rework series lined up and ready to go. Is there any other reason not
to do this, given my previous reply?

Stephen

> > Do you have a plan to avoid losing the git history if you do this move?
> 
> Yup, I've already pulled this out (strictly as a test, until we make a
> decision here). The history is retained [1] and I've documented the
> steps in the repo itself [2]. I've also pushed up some of the future
> changes I have prepared [3], which see us breaking up this monolithic
> file into smaller chunks, in preparation for the eventual addition of
> REST API support along with some smaller fixes.
> 
> > I am somewhat biased because I almost always just invoke pwclient from
> > the project directory as patchwork/bin/pwclient, so this would totally
> > break my usecase, but I realise that's not a valid reason per se.
> 
> Stephen
> 
> > Regards,
> > Daniel
> 
> PS: As an aside, I do think this shows an interesting difference in our
> backgrounds shows up :) I'm used to working with multiple different
> repos in OpenStack, where everything is split out into smaller units
> and versioned APIs ensure different components talk to each other. As
> such, splitting this out feels natural to me. However, I guess the
> kernel, with it's monolithic repo, does things differently and I'm
> guessing tooling for that is all kept in-tree? Interesting, in any
> case.
> 
> [1] https://github.com/getpatchwork/pwclient/commits/master
> [2] https://github.com/getpatchwork/pwclient/commit/23fd64ad3
> [1] https://github.com/getpatchwork/pwclient/commits/devel
> 
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

Stephen Finucane <stephen@that.guru> writes:

> On Mon, 2018-10-22 at 16:55 +0100, Stephen Finucane wrote:
>> On Tue, 2018-10-23 at 01:31 +1100, Daniel Axtens wrote:
>> > Stephen Finucane <stephen@that.guru> writes:
>> > 
>> > > Let's start managing this via a separate project, which will allow the
>> > > client to evolve separately from the server. No redirect is added for
>> > > the old '/pwclient' URL as it seems wiser to return a HTTP 404 error
>> > > code.
>> 
>> So...
>> 
>> > I can't say I'm thrilled by the idea. It adds complexity and I don't
>> > know what it would make easier. Presumably patches for it would still go
>> > to this list and be reviewed in the usual way, so I'm not sure how it
>> > would help workflow.
>> 
>> Yup, I plan on keeping development on the same mailing list, if at all
>> possible (more on this in a bit).
>> 
>> > It's also a single file at the moment, so it's not like it's so
>> > complex that it should be spun out to simplify things...
>> 
>> But this is changing. There are two things I'd like to do here that
>> make shipping this separately seem like a a smarter thing.
>> 
>>  * I want to start shipping pwclient on PyPI so I can just 'pip
>>    install' wherever I need it (and maybe 'dnf install', in the
>>    future).
>> 
>>  * I want to add REST API support but would rather not duplicate the
>>    underlying client in git-pw and pwclient (and whatever other clients
>>    may be developed in the future). To this end, I'd like to develop a
>>    'pwlib' library that will provide a generic API client, meaning
>>    'pwclient' and 'git-pw' will simply provide the user chrome.
>> 
>> These two points are related. Assuming we ship this on PyPI, we're
>> going to need to add some of distribution tooling (pbr, in this case,
>> though setuptools or plain old distutils would also be valid options).
>> I'll also distribution tooling if I want to follow through on 'pwlib'
>> plan, both to allow 'pwlib' itself to be distributed via PyPI and so
>> 'pwclient' can pull this in as part of it's installation (I really
>> don't want to vendor pwlib in-tree). All of this means copy-pasting the
>> script will no longer be an option and the single file we currently
>> have will grow into multiple files before long. While could keep
>> developing pwclient (and optionally pwlib) in-tree, doing so makes life
>> harder for us with regard to things like versioning, documentation,
>> distribution, etc. Similarly, while we could get away with it for
>> single file script, it seems wrong to package multiple complex projects
>> within the same tree, each with their own versioning, documentation,
>> and release processes. The server will always take priority, meaning
>> things like tags, the 'docs' directory, 'tox' files etc. will apply
>> first and foremost to that project.
>> 
>> 'git-pw' is already its own project and I think this has allowed it to
>> develop faster than it might have done were it found in tree. The only
>> thing I haven't done with this is develop it fully on the list (mostly
>> because I think of it as a third-party sort of tool) but we can
>> definitely continue to develop pwclient on list. In fact, the Patchwork
>> 2.1 'project.subject_match' feature allows us to this (and gives us a
>> way to dog food the feature).
>
> Now that the ozlabs instance has been updated to v2.1, we have access
> to the subject_match feature and so I'd like to proceed with this
> migration. The 'patchwork/pwclient' repo is seeded and I have a large
> rework series lined up and ready to go. Is there any other reason not
> to do this, given my previous reply?

Yep, go ahead.

I still don't love it as an approach, but I also want to make sure
pwclient development happens and especially that we can kill off xmlrpc,
and this is clearly the best way to free you up to do it.

I'd be keen for us to package it as a deb and/or a snap as well for the
benefit of those of us not in Fedoraland. Remind me to do this at some
point :)

Regards,
Daniel

>
> Stephen
>
>> > Do you have a plan to avoid losing the git history if you do this move?
>> 
>> Yup, I've already pulled this out (strictly as a test, until we make a
>> decision here). The history is retained [1] and I've documented the
>> steps in the repo itself [2]. I've also pushed up some of the future
>> changes I have prepared [3], which see us breaking up this monolithic
>> file into smaller chunks, in preparation for the eventual addition of
>> REST API support along with some smaller fixes.
>> 
>> > I am somewhat biased because I almost always just invoke pwclient from
>> > the project directory as patchwork/bin/pwclient, so this would totally
>> > break my usecase, but I realise that's not a valid reason per se.
>> 
>> Stephen
>> 
>> > Regards,
>> > Daniel
>> 
>> PS: As an aside, I do think this shows an interesting difference in our
>> backgrounds shows up :) I'm used to working with multiple different
>> repos in OpenStack, where everything is split out into smaller units
>> and versioned APIs ensure different components talk to each other. As
>> such, splitting this out feels natural to me. However, I guess the
>> kernel, with it's monolithic repo, does things differently and I'm
>> guessing tooling for that is all kept in-tree? Interesting, in any
>> case.
>> 
>> [1] https://github.com/getpatchwork/pwclient/commits/master
>> [2] https://github.com/getpatchwork/pwclient/commit/23fd64ad3
>> [1] https://github.com/getpatchwork/pwclient/commits/devel
>> 
>> _______________________________________________
>> Patchwork mailing list
>> Patchwork@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/patchwork

Hello Daniel,

Am 11.06.19 um 03:31 schrieb Daniel Axtens:

> I'd be keen for us to package it as a deb and/or a snap as well for the
> benefit of those of us not in Fedoraland. Remind me to do this at some
> point :)

years ago I intended to package patchwork for Debian.

https://bugs.debian.org/703226

But because of time constrains and knowledge limitations on Django and
the amount of time needed for creating the additional needed packages
for setting up the various types of databases and the update scenarios I
have given up.

I'm happy to sponsor at some time an upload to Debian if a significant
contribution for future updates to the packaging can be ensured.
Currently I haven't a need for getting a patchwork setup running for an
productive system and I also haven't looked into patchwork and the
details for a long time. But shouldn't be that hard to create some basic
working setup within a packaging.

Anybody can contact me of list to discuss some things needed to be clear.