Upgrade patchbot to 2.6.1 as a system package

[chapoton]

The patchbot is currently installing in a very not kosher way, as explained in

​https://github.com/robertwb/sage-patchbot/issues/79

This ticket will change completely the installation mode. The patchbot will become a python package, to be installed using pip in the system python as explained below.

This ticket also updates the patchbot, in particular to cope with the new https trac.

New tarball for patchbot: ​http://www-irma.u-strasbg.fr/~chapoton/sage-patchbot-2.6.1.tar.gz

installation (two equivalent methods at your disposal):

1. Download the tarball and then

   pip install --user sage-patchbot-2.6.1.tar.gz
   

2. (recommended)

   pip install --user git+https://github.com/sagemath/sage-patchbot.git@2.6.1
   

There are now two possible ways to launch the patchbot: 1.

python -m sage_patchbot.patchbot --sage-root=XXXX

or

2. Checkout the branch here and run

   make && ./sage --patchbot
   

[chapoton]

I tried and failed so far. Help is required, please.

---

New commits:

​a827087

trac 20736 first try, failing

[embray]

Not related to installation problem, but in the dependencies file you should put

$(PYTHON) | setuptools

That is, if setuptools is reinstalled it's not necessary to reinstall a Python package that happens to use it. But it is a prerequisite to installing.

[git]

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

​c9907ef

trac 20736 change in dependencies

[embray]

BTW, when building a source package of a Python project, use ./setup.py sdist to make the tarball (also make sure the tarball contains all the files it should contain, and none that it shouldn't contain--you might have to edit a MANIFEST.in file to adjust it a la ​https://docs.python.org/2/distutils/sourcedist.html#specifying-the-files-to-distribute)

[git]

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

​f3839fd

another try, failing the same way

[chapoton]

I am making some slow progress.

[jdemeyer]

There is a fundamental problem here, mentioned also at ​https://github.com/robertwb/sage-patchbot/issues/79

We cannot use the Sage Python for the patchbot, since the patchbot might reinstall Python. So I don't know what the best solution is, but installing it "the usual way" is not the right thing to do here.

[git]

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

​160332c

fixing the spkg-install script

[embray]

So it's a build dep for Sage and should be installed "the usual way" using the system Python.

[jdemeyer]

Replying to embray:

So it's a build dep for Sage and should be installed "the usual way" using the system Python.

I don't get why you say a "build dep", but I agree with the second part.

[chapoton]

I changed the name of the package back to sage-patchbot. But I am little bit stuck here.

It kind of worked with the package name "patchbot", but it was installed in SAGE_LOCAL.

[embray]

Replying to jdemeyer:

Replying to embray:

So it's a build dep for Sage and should be installed "the usual way" using the system Python.

I don't get why you say a "build dep", but I agree with the second part.

"build dep" is probably the wrong description--but I just meant a developer tool for development of Sage, outside the sage distribution.

[embray]

Replying to chapoton:

I changed the name of the package back to sage-patchbot. But I am little bit stuck here.

It kind of worked with the package name "patchbot", but it was installed in SAGE_LOCAL.

To be clear, this is a case where the term "package" is often overloaded in Python:

• Here the package, as in the name of what you import in Python is patchbot

• The name in the setup.py is different from the package name, and as sometimes referred to as the "distribution name" or the "PyPI name" to reduce confusion. Often the package name and the distribution name are the same, but they don't have to be. Either way it shouldn't be relevant here.

I think the main takeaway here is that the patchbot should not be a Sage spkg at all. It's not a part of sage, the distribution. It's a tool for testing sage the distribution and as such should be entirely external to it.

So to install the patchbot on say a Linux system one would use sudo pip install <path/to/tarball> or even just sudo pip install sage-patchbot if the latest version is uploaded to PyPI.

Alternatively this can be done within a virtualenv if we prefer to avoid the system install.

[chapoton]

New commits:

​eed116f	Merge branch 'public/20736' into 7.3.b4
​228842b	update of patchbot to 2.6.0

[chapoton]

This is becoming urgent, to cope with https trac.

I tried to go back to the previous way but this is not working. It complains about

sage -patchbot --ticket=20862
Traceback (most recent call last):
  File "sage/local/bin/patchbot/patchbot.py", line 63, in <module>
    from .trac import scrape, pull_from_trac, TracServer, Config
ValueError: Attempted relative import in non-package

I need help. Should I just remove all relative imports newly introduced ?? should I keep the strange source dir of patchbot inside the local/bin directory ??

[chapoton]

I am trying a new way:

• have a tarball made with "python ./setup.py sdist"
• install it using "pip install sage-patchbot-tarball"
• run it as usual by "sage -patchbot" (with sage on the current branch)

[jdemeyer]

Replying to chapoton:

usage: sudo pip install sage-patchbot-2.6.0.tar.gz

VERY -1 to this!

---

New commits:

​91fc492

patchbot : trying to use pip install

[jdemeyer]

Can you please update the patchbot to only deal with the https thing and not change the whole install procedure?

[chapoton]

I am doing what I can, but this is a bit too complicated for me to handle easily.

I changed all the imports in such a way that patchbot must now be used a a python package. The simpplest way (and it works) is what I suggest above.

Why do you so strongly object to pip installation in the system python ?

Now if you have better suggestions, I would appreciate help.

[jdemeyer]

Replying to chapoton:

I changed all the imports in such a way that patchbot must now be used a a python package.

I don't understand how the imports should be related to how the package is installed.

Why do you so strongly object to pip installation in the system python ?

It should be possible to install and run the patchbot without sudo access.

[chapoton]

Replying to jdemeyer:

Replying to chapoton:

I changed all the imports in such a way that patchbot must now be used a a python package.

I don't understand how the imports should be related to how the package is installed.

Relative imports are not allowed when a python file is run as a script, it seems.

Why do you so strongly object to pip installation in the system python ?

It should be possible to install and run the patchbot without sudo access.

How is it compatible with not installing in the sage python ?

Is there any way to install somewhere inside sage a python package using the system python ? Is there any existing case ? Where should it go ? How should I do that ?

[jdemeyer]

Replying to chapoton:

Relative imports are not allowed when a python file is run as a script, it seems.

That can't be true. You're probably just missing some __init__.py file.

[jdemeyer]

Replying to chapoton:

Is there any way to install somewhere inside sage a python package using the system python ? Is there any existing case ? Where should it go ? How should I do that ?

User installs are possible with pip, I edited the ticket description.

[jdemeyer]

If the patchbot becomes a system package, it should be removed as Sage package. I.e. remove the whole directory build/pkgs/patchbot.

[chapoton]

Replying to jdemeyer:

If the patchbot becomes a system package, it should be removed as Sage package. I.e. remove the whole directory build/pkgs/patchbot.

yes, I agree.

And there is an __init__.py in the package.

[jdemeyer]

Replying to chapoton:

And there is an __init__.py in the package.

Well, if you want me to have a look, I need more details. But if it works as intended outside of Sage with a user install, that's good enough.

---

New commits:

​897f5b2

trac 20736 removing patchbot as spkg

[embray]

If installed correctly there should be no sage/local/bin/patchbot/patchbot.py

The exception you're getting should be unrelated to the new patchbot package. Instead, the old patchbot spkg should *not* be getting installed.

[embray]

Replying to jdemeyer:

Replying to chapoton:

usage: sudo pip install sage-patchbot-2.6.0.tar.gz

VERY -1 to this!

---

New commits:

​91fc492

patchbot : trying to use pip install

+1 to this. This is an appropriate way to install a Python package that is not (and will likely never be) provided by the system packager. Debian, at least, handles this well.

The alternative would be some kind of ~/.local install--need to ensure that the PYTHONPATH is correct when running the patchbot though.

As I've written before on this thread the patchbot should not be a Sage spkg.

[chapoton]

The 2.6.0 seems to work well enough on sage4, although at some point it tested closed tickets.

On the other hand, arando is uselessly running 2.5.7.

[jdemeyer]

Replying to chapoton:

On the other hand, arando is uselessly running 2.5.7.

I know, I have not upgraded it yet.

[jdemeyer]

Replying to chapoton:

The 2.6.0 seems to work well enough on sage4, although at some point it tested closed tickets.

I did have to manually change PYTHONPATH for it to work in a user install.

I think you should be able to do

./sage --pip install --user patchbot

However, since Sage sets PYTHONUSERBASE (no idea why), this patchbot is installed in a directory different from what the system Python expects. To get it working, I had to set PYTHONPATH to the site-packages directory in ~/.sage/local. Because of confusion between lib and lib64, just setting PYTHONUSERBASE system-wide was not sufficient.

For this reason, I believe that ./sage --patchbot should set PYTHONPATH before running the patchbot.

[embray]

I don't know why you would run ./sage --patchbot at all, as opposed to just patchbot. Why is it necessary to use sage to run the thing that's supposed to be testing sage?

[chapoton]

I propose the following "modus operandi" for patchbot installation and use

• get the latest tarball.tar.gz from somewhere (git or my web page?)
• pip install --user tarball.tar.gz
• sage -patchbot --options (1) OR python -m sage_patchbot.patchbot --options (2)

Everything (pip and python) should take place in the system python. I do not see any reason to involve the inner python of sage.

I am not completely sure if this already works with this branch, but it was my vision of things. I think that I did (1) or (2) on my own patchbot client, which seems to be running well.

Now Jeroen and Eric do not seem to agree on what to do. Please consider that I have not a lot of time to spend on that.

[chapoton]

much more annoying is the fact that the patchbot persist to test closed tickets..

[embray]

get the latest tarball.tar.gz from somewhere (git or my web page?)

Why not put it on PyPI, and put out some proper releases?

Alternatively, pip allows download direct from a git repository, including specific tags.

pip install --user tarball.tar.gz

Whatever; people can pip install however they're comfortable with.

Everything (pip and python) should take place in the system python. I do not see any reason to involve the inner python of sage.

+1

I am not completely sure if this already works with this branch, but it was my vision of things. I think that I did (1) or (2) on my own patchbot client, which seems to be running well.

I don't know. Is there anything in the patchbot that tries to import sage? I don't think there is--as far as I can tell the patchbot can run entirely outside of the sage environment, as it should.

Now Jeroen and Eric do not seem to agree on what to do. Please consider that I have not a lot of time to spend on that.

AFAICT the only disagreement is over whether or not running sudo pip install is ever appropriate. My point is that it doesn't really matter--as long as the user is installing into a Python outside of the Sage distribution (be it a virtualenv, a --user install, or whatever) it should work fine. It doesn't really matter and the most appropriate choice depends on the context.

For that matter, there's probably nothing wrong with installing the patchbot package into Sage's Python too, again just using pip install. But in general it should not be necessary to.

[chapoton]

Let us try to move forward, please. This works, as far as I can tell.

The patchbot package is installed in the system python, not as root but as user.

I would prefer not to use pypi for the moment.

Could somebody try to follow exactly the instructions and see if everything runs smoothly ?

[vdelecroix]

If I install the package on my system Python, then doing

$ python -m sage_patchbot.patchbot

I got

Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/nfs4/home6/vdelecro/.local/lib/python2.7/site-packages/sage_patchbot/patchbot.py", line 1471, in <module>
    main(args)
  File "/nfs4/home6/vdelecro/.local/lib/python2.7/site-packages/sage_patchbot/patchbot.py", line 1401, in main
    patchbot = Patchbot(os.path.abspath(options.sage_root),
  File "/usr/lib/python2.7/posixpath.py", line 367, in abspath
    if not isabs(path):
  File "/usr/lib/python2.7/posixpath.py", line 61, in isabs
    return s.startswith('/')
AttributeError: 'NoneType' object has no attribute 'startswith'

[vdelecroix]

All right, the command *must* specify the sage root as

$ python -m sage_patchbot.patchbot --sage-root=WHATEVER

(or setting SAGE_ROOT environment variable). I think there should be a more specific error message than what I got in comment:43. Note that if the package is installed in Sage python then it will work out of the box since SAGE_ROOT is defined.

I don't understand why you do not want to put it on PyPI. I can do it myself if you do not want to investigate. But it really takes less than a minute and greatly simplify the installation, version upgrade, online documentation, ...

[vdelecroix]

(I launch the package version of the patchbot on librae)

[vdelecroix]

I guess that the log should not go under SAGE_ROOT/logs/patchbot anymore now. The path should be part of the configuration file. What do you think?

[vdelecroix]

What about switching to ​logging Python module?

[chapoton]

Replying to vdelecroix:

If I install the package on my system Python, then doing

$ python -m sage_patchbot.patchbot

Thanks for the feedback.

These are not the instructions on how to run the patchbot given in the ticket description.

But this will be another possible way.

Librae is meeting a whole bunch of doctest failure with lrslib. Maybe it should be checked that the bot is not causing them.

Concerning pypi, it may be simpler once done, but I am not really ready to add yet another layer of complexity.

Concerning the log, I would prefer to postpone the problem, once the new setting is in place.

[embray]

Replying to vdelecroix:

I don't understand why you do not want to put it on PyPI. I can do it myself if you do not want to investigate. But it really takes less than a minute and greatly simplify the installation, version upgrade, online documentation, ...

I don't think it's deeply necessary--pip can install fine from a git repository for example (including specific tags). But it's not a big deal either way.

And like I also wrote earlier it shouldn't matter where pathbot is installed either as long as it's installed correctly and running patchbot from the command-line works.

[embray]

Replying to chapoton:

Concerning pypi, it may be simpler once done, but I am not really ready to add yet another layer of complexity.

You've already fixed up the patchbot to use distutils properly, so this is a matter of making an account on PyPI, setting up your credentials, and running ./setup.py register upload. There's no additional layer of complexity.

[vdelecroix]

Replying to chapoton:

Replying to vdelecroix:

If I install the package on my system Python, then doing

$ python -m sage_patchbot.patchbot

Thanks for the feedback.

These are not the instructions on how to run the patchbot given in the ticket description.

But this will be another possible way.

Indeed. If the patchbot is a Python package it is much more natural to use it without the Sage environment variables set.

Librae is meeting a whole bunch of doctest failure with lrslib. Maybe it should be checked that the bot is not causing them.

Not the bot: #21012

Concerning pypi, it may be simpler once done, but I am not really ready to add yet another layer of complexity.

It is not a new layer. It is simply a way to distribute the module. Instead of having a tarball on your webpage there is a tarball on PyPI. It is just simpler because for installation you would just have to do pip install sage-patchbot --user (downloading would be automatic). And for upgrade pip install sage-patchbot --upgrade.

Concerning the log, I would prefer to postpone the problem, once the new setting is in place.

Of course this was not intended to be for this ticket. Still it is a question.

[vdelecroix]

The patchbot librae is hanging on #19092 since more than a day

The current date is Fri Jul 22 21:25:47 CEST 2016. The end of logs/patchbot/patchbot.log

[2016-07-21 23:38:46] Getting ticket list...
[2016-07-21 23:39:22] testing found ticket #19092
[2016-07-21 23:39:22] #19092: init phase

last lines in logs/install.log

[dochtml] [tutorial ] copying extra files... done
[dochtml] [tutorial ] dumping search index in German (code: de) ... done
[dochtml] [tutorial ] dumping object inventory... done
[dochtml] [tutorial ] build succeeded.
[dochtml] Build finished.  The built documents can be found in /home/vincent/sage_patchbot/local/share/doc/sage/html/de/tutorial

After a Ctrl-C I had the following traceback (appearing only in logs/install.log)

[dochtml] Process PoolWorker-173:
[dochtml] Process PoolWorker-178:
[dochtml] Process PoolWorker-174:
[dochtml] Process PoolWorker-177:
[dochtml] Traceback (most recent call last):
[dochtml] Traceback (most recent call last):
[dochtml] Traceback (most recent call last):
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 258, in _bootstrap
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 258, in _bootstrap
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 258, in _bootstrap
[dochtml] Process PoolWorker-176:
[dochtml] Process PoolWorker-175:
[dochtml] Traceback (most recent call last):
[dochtml] Traceback (most recent call last):
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 258, in _bootstrap
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 258, in _bootstrap
[dochtml] Traceback (most recent call last):
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 258, in _bootstrap
[dochtml]     self.run()
[dochtml]     self.run()
[dochtml]     self.run()
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 114, in run
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 114, in run
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 114, in run
[dochtml]     self.run()
[dochtml]     self._target(*self._args, **self._kwargs)
[dochtml]     self._target(*self._args, **self._kwargs)
[dochtml]     self._target(*self._args, **self._kwargs)
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 114, in run
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/pool.py", line 102, in worker
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/pool.py", line 102, in worker
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/pool.py", line 102, in worker
[dochtml]     self.run()
[dochtml]     self._target(*self._args, **self._kwargs)
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 114, in run
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/pool.py", line 102, in worker
[dochtml]     self._target(*self._args, **self._kwargs)
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/pool.py", line 102, in worker
[dochtml]     self.run()
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/process.py", line 114, in run
[dochtml]     self._target(*self._args, **self._kwargs)
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/pool.py", line 102, in worker
[dochtml]     task = get()
[dochtml]     task = get()
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/queues.py", line 376, in get
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/queues.py", line 376, in get
[dochtml]     task = get()
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/queues.py", line 376, in get
[dochtml]     task = get()
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/queues.py", line 376, in get
[dochtml]     task = get()
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/queues.py", line 376, in get
[dochtml]     task = get()
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/queues.py", line 378, in get
[dochtml]     return recv()
[dochtml]   File "src/cysignals/signals.pyx", line 225, in cysignals.signals.python_check_interrupt (build/src/cysignals/signals.c:2429)
[dochtml]     racquire()
[dochtml]     racquire()
[dochtml]   File "src/cysignals/signals.pyx", line 225, in cysignals.signals.python_check_interrupt (build/src/cysignals/signals.c:2429)
[dochtml]   File "src/cysignals/signals.pyx", line 225, in cysignals.signals.python_check_interrupt (build/src/cysignals/signals.c:2429)
[dochtml]     racquire()
[dochtml]     racquire()
[dochtml]   File "src/cysignals/signals.pyx", line 225, in cysignals.signals.python_check_interrupt (build/src/cysignals/signals.c:2429)
[dochtml]   File "src/cysignals/signals.pyx", line 225, in cysignals.signals.python_check_interrupt (build/src/cysignals/signals.c:2429)
[dochtml]     racquire()
[dochtml]   File "src/cysignals/signals.pyx", line 225, in cysignals.signals.python_check_interrupt (build/src/cysignals/signals.c:2429)
[dochtml]   File "src/cysignals/signals.pyx", line 96, in cysignals.signals.sig_raise_exception (build/src/cysignals/signals.c:1116)
[dochtml]   File "src/cysignals/signals.pyx", line 96, in cysignals.signals.sig_raise_exception (build/src/cysignals/signals.c:1116)
[dochtml]   File "src/cysignals/signals.pyx", line 96, in cysignals.signals.sig_raise_exception (build/src/cysignals/signals.c:1116)
[dochtml]   File "src/cysignals/signals.pyx", line 96, in cysignals.signals.sig_raise_exception (build/src/cysignals/signals.c:1116)
[dochtml]   File "src/cysignals/signals.pyx", line 96, in cysignals.signals.sig_raise_exception (build/src/cysignals/signals.c:1116)
[dochtml] KeyboardInterrupt
[dochtml] KeyboardInterrupt
[dochtml]   File "src/cysignals/signals.pyx", line 96, in cysignals.signals.sig_raise_exception (build/src/cysignals/signals.c:1116)
[dochtml] KeyboardInterrupt
[dochtml] KeyboardInterrupt
[dochtml] KeyboardInterrupt
[dochtml] KeyboardInterrupt
[dochtml] Error building the documentation.
[dochtml] Traceback (most recent call last):
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/runpy.py", line 162, in _run_module_as_main
[dochtml]     "__main__", fname, loader, pkg_name)
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/runpy.py", line 72, in _run_code
[dochtml]     exec code in run_globals
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__main__.py", line 2, in <module>
[dochtml]     main()
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 1630, in main
[dochtml]     builder()
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 298, in _wrapper
[dochtml]     x.get(99999)
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/pool.py", line 561, in get
[dochtml]     self.wait(timeout)
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/multiprocessing/pool.py", line 556, in wait
[dochtml]     self._cond.wait(timeout)
[dochtml]   File "/home/vincent/sage_patchbot/local/lib/python/threading.py", line 359, in wait
[dochtml]     _sleep(delay)
[dochtml]   File "src/cysignals/signals.pyx", line 225, in cysignals.signals.python_check_interrupt (build/src/cysignals/signals.c:2429)
[dochtml]   File "src/cysignals/signals.pyx", line 96, in cysignals.signals.sig_raise_exception (build/src/cysignals/signals.c:1116)
[dochtml] KeyboardInterrupt
make[1]: *** [doc-html] Error 1

[vdelecroix]

Maybe due to swap (I need to check)...

[jdemeyer]

Replying to embray:

I don't know why you would run ./sage --patchbot at all

Mainly just for convenience. It's easier to type ./sage --patchbot than python -m sage_patchbot.patchbot --sage-root=SAGE_ROOT.

Second reason: on most systems, python is installed system-wide. But pip often is not.

[jdemeyer]

Please update the ticket description with the recommended way to install and run the patchbot. Then I will test it.

[jdemeyer]

You should also decide whether you want to support ./sage --patchbot or not. If you do, it should actually work (see comment:37). If you don't, remove the command (or better: make it print some message of which commands should actually be run).

[chapoton]

@jdemeyer

I have tried to enhance the description of the ticket.

I am not willing to avoid the dependency to pip.

[jdemeyer]

Like I said before, ./sage --patchbot does not actually work.

[jdemeyer]

See the last line of 37

[vdelecroix]

Indeed the command ./sage --patchbot has no clue where the patchbot is actually installed. It might be on system python, in Sage python, in a virtualenv, or something else... I think it is much safer to disable it completely with an error message (as proposed in comment:56).

[jdemeyer]

I think it makes sense to assume that, if you run ./sage --patchbot, you want to run the patchbot which is installed from a Sage shell. That would just require some part of sage-env to be executed before running the patchbot.

[git]

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

​80a58ba	Merge branch 'public/20736' in 7.3.rc0
​14fb599	trac 20736 more information when calling sage --patchbot

[chapoton]

It seems that we are not making progress here. Jeroen, why do you insist so strongly on installing the patchbot package inside the sage shell ? It makes not much sense to me. I think one can reasonably expect anybody wanting to run a patchbot to be able to install pip first. There is no need to call "sage -pip".

I propose the following alternative:

(1) disable completely "sage --patchbot"

(2) use "sage --patchbot" as a convenient way to launch the patchbot installed in the system python.

I vote for (2), which is the current situation with the branch. I have just added some explanation text to appear when "sage --patchbot" is called.

[jdemeyer]

Jeroen, why do you insist so strongly on installing the patchbot package inside the sage shell ?

I am not really insisting on that. I am insisting on not requiring a system install. But I can live with a user install outside of Sage.

[jdemeyer]

To turn the question around: why do you keep insisting on not supporting a patchbot installed within Sage?

[chapoton]

I do not insist on that either, I just do not know how to do it, and I am happy enough with the current state of things. I would prefer to stop spending my time on this ticket, and to get more momentum towards python 3 instead.

[vdelecroix]

It is supported with

$ sage -pip install my_patchbot_tarball.tar.gz
$ sage -python -m sage_patchbot.patchbot --sage-root=my_sage_root

It is just the command sage -patchbot which is half broken. So I would vote for (1) that is much cleaner and solve the issue.

EDIT: in my initial post I wrote (2) instead of (1)... sorry

[chapoton]

Jeroen, do you also prefer (1) ?

[jdemeyer]

Replying to vdelecroix:

It is supported with

$ sage -pip install my_patchbot_tarball.tar.gz
$ sage -python -m sage_patchbot.patchbot --sage-root=my_sage_root

No, that does not really work since you cannot use the Sage Python to run the patchbot.

[vdelecroix]

Replying to jdemeyer:

Replying to vdelecroix:

It is supported with

$ sage -pip install my_patchbot_tarball.tar.gz
$ sage -python -m sage_patchbot.patchbot --sage-root=my_sage_root

No, that does not really work since you cannot use the Sage Python to run the patchbot.

Why is that? It (seems to) work for me.

[jdemeyer]

Replying to chapoton:

I do not insist on that either, I just do not know how to do it

Good. I will look into this then.

[git]

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

​654e578

Improve --patchbot command

[jdemeyer]

First attempt to support a patchbot installed within Sage, not fully tested yet.

[jdemeyer]

Replying to vdelecroix:

Why is that? It (seems to) work for me.

...until the patchbot tests a ticket which re-installs the Python within Sage.

[jdemeyer]

Hang on, I have a better solution in mind to run the patchbot.

[git]

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

​6d7990e

Improve --patchbot command

[jdemeyer]

Currently testing...

[jdemeyer]

This is good for me. Can anybody review my commit?

[chapoton]

does not work for me:

0) pull the branch here and make
1) pip install --user git+https://github.com/sagemath/sage-patchbot.git@2.6.1
2) ./sage --patchbot
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: No module named sage_patchbot
Error: cannot find installation path for sage_patchbot
See https://wiki.sagemath.org/buildbot/details for instructions

but "import sage_patchbot" works in the system python

[vdelecroix]

Instead of a sloppy script, I would be very much happier with sage -patchbot returning an informative error message.

[embray]

Replying to vdelecroix:

Instead of a sloppy script, I would be very much happier with sage -patchbot returning an informative error message.

+1

As I understand it the purpose of ./sage -foo is to run the foo program that's installed in the sage environment. I don't see why it should be doing anything else. At most if ./sage -patchbot is meant to do something it can raise an error if patchbot does not happen to be installed in the Sage environment. Then if one wants they can ./sage -pip install sage-patchbot and get it in their Sage environment, at which point ./sage -patchbot is the equivalent of . sage-env; patchbot, etc...

Otherwise all one needs to do to run the patchbot is to run patchbot however it happens to be installed on a (correctly configured) system.

[rws]

So what exactly is necessary to make this branch work, i.e. to run sage -patchbot successfully?

[rws]

Answer: ./sage -pip install sage-patchbot or the downloaded 2.6.1 tarball as argument. This means the wiki instructions need to change which I just did, please check.

[chapoton]

Please do not interfere with the wiki instructions until this ticket is closed. Revert your changes there, please.

[chapoton]

Thanks. The current branch should be considered as broken. The wiki instructions were perfectly working until Jeroen made his new branch.

EDIT: a working branch is at u/chapoton/20736

[jdemeyer]

Replying to rws:

So what exactly is necessary to make this branch work, i.e. to run sage -patchbot successfully?

That's easy: I just need to check both the Python-in-Sage and the system Python.

However, there seems to be disagreement (see @vdelecroix and @embray) about what ./sage --patchbot should actually do. There is no point in fixing the branch if I have to throw it away anyway.

[chapoton]

Proposed solution:

• do not allow sage -patchbot for running the bot
• make so that sage -patchbot just prints:

  The patchbot can no longer be run in this way.
  Please see https://wiki.sagemath.org/buildbot/details for up-to-date instructions.
  

I would accept this solution.

[jdemeyer]

My proposal would be to fix ./sage --patchbot to work both with a patchbot installed in Sage as well as a patchbot installed system-wide. This is easy to do, but it would result in what Vincent calls "a sloppy script".

[jdemeyer]

I think the goal should be to make it as easy as possible to install and run the patchbot. We could even make it such that ./sage -i patchbot && ./sage --patchbot still works as before.

[chapoton]

I think it is more simple to propose a unique way to install and run the patchbot

• install in the system python using pip
• run using python -m

So that people have no choice to make. And this is still only two command lines.

[jdemeyer]

Replying to chapoton:

And this is still only two command lines.

Actually, it's five commands:

1. Download pip
2. Install pip
3. Download sage_patchbot
4. Install sage_patchbot
5. Run sage_patchbot

[chapoton]

Meaning no offense, this comment is slightly small-minded, IMHO

[embray]

Let's assume for a second that users are intelligent enough to run commands on their system that are not wrapped in Sage, and that they have working systems with working Python installations...

What advantage is there to

$ ./sage -patchbot

over

$ patchbot

[jdemeyer]

Replying to embray:

that they have working systems with working Python installations...

A working Python installation is not the same as a working pip installation.

What advantage is there to

$ ./sage -patchbot

over

$ patchbot

The fact that ./sage --patchbot does not require the user to change his $PATH.

[embray]

Replying to jdemeyer:

Replying to embray:

that they have working systems with working Python installations...

A working Python installation is not the same as a working pip installation.

It effectively is. pip is the official installer for Python packages and comes with modern distributions of Python (including most system distributions). While you can have a "working" Python installation without pip, a Python installation with a non-working pip installation is not a working Python installation.

What advantage is there to

$ ./sage -patchbot

over

$ patchbot

The fact that ./sage --patchbot does not require the user to change his $PATH.

Why would a user need to change their path?

[jdemeyer]

The sage4 patchbot is broken:

[2016-09-08 09:56:55] Getting ticket list...
Traceback (most recent call last):
  File "/var/worker.sage/local/lib/python2.7/site-packages/sage_patchbot/patchbot.py", line 1470, in main
    patchbot.test_a_ticket(ticket)
  File "/var/worker.sage/local/lib/python2.7/site-packages/sage_patchbot/patchbot.py", line 951, in test_a_ticket
    rating, ticket = self.get_one_ticket()
  File "/var/worker.sage/local/lib/python2.7/site-packages/sage_patchbot/patchbot.py", line 774, in get_one_ticket
    return all_tickets[-1]
IndexError: list index out of range

[jdemeyer]

There is something very wrong in how the patchbot gets its tickets from the server...

[chapoton]

Replying to jdemeyer:

There is something very wrong in how the patchbot gets its tickets from the server...

That's probably true. But your specific problem with sage4 is also that it has already recently given a report on every available ticket with "BuildFailed?" status.

[jdemeyer]

Replying to chapoton:

But your specific problem with sage4 is also that it has already recently given a report on every available ticket with "BuildFailed?" status.

Right, that is #21430. But still, that should be completely independent from how the patchbot itself works.

[git]

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

​a3b040f	Set JUPYTER_CONFIG_DIR to ~/.sage/jupyter and hardcode IPYTHONDIR
​78f964e	Clarify comment (hopefully)
​fd3ab44	Update patchbot to 2.6.1 as system package
​fc8bcec	Improve --patchbot command

[jdemeyer]

Rebased over #21430, partially squashed, fixed installations in system Python.

[jdemeyer]

I am currently running the patchbot on sage4 and arando with this branch.

[klee]

If you guys are not on agreement, then I am +1 on removing or deprecating sage --patchbot. As a recent volunteer to run a patchbot, I thought it is illogical to invoke Sage to run a patchbot whose mission is to test Sage. Installing patchbot by "pip ..." and then running "python -m ..." is straightforward and simple. Why do we need an alternate and conceptually confusing way to launch a patchbot?

If this disagreement persists, then why don't we have a poll on sage-devel?

By the way, this pip install --user git+https://github.com/sagemath/sage-patchbot.git installs the latest patchbot, why does the statement in the wiki specify the version, which would be outdated sooner or later?

[jdemeyer]

Replying to klee:

If you guys are not on agreement, then I am +1 on removing or deprecating sage --patchbot. As a recent volunteer to run a patchbot, I thought it is illogical to invoke Sage to run a patchbot whose mission is to test Sage.

I don't see it as invoking Sage. I consider sage --patchbot just a simple convenience command. I changed it such that it should work in all cases: the sage_patchbot can be installed system-wide, by the user, or inside Sage. Why should we not support that?

Installing patchbot by "pip ..."

Then pip becomes an additional requirement for running the Sage patchbot. Many systems come with Python installed system-wide, but without pip.

[klee]

I don't see it as invoking Sage. I consider sage --patchbot just a simple convenience command.

You can see it as such since you know the internals. But for those who do not know the details of how patchbot works, like me, the command gives the impression that I am about to run a patchbot installed inside Sage.

On the other hand, It is weird to control the patchbot installed in the system Python via sage interface.

[jdemeyer]

Replying to klee:

the command gives the impression that I am about to run a patchbot installed inside Sage.

Which might be the case and which is not a problem (note however that it will always be run using the system Python).

[embray]

Replying to klee:

I don't see it as invoking Sage. I consider sage --patchbot just a simple convenience command.

You can see it as such since you know the internals. But for those who do not know the details of how patchbot works, like me, the command gives the impression that I am about to run a patchbot installed inside Sage.

On the other hand, It is weird to control the patchbot installed in the system Python via sage interface.

I agree. I think "invoking sage" here should be read as "activating the sage environment" which is true. There's no more reason to run sage --patchbot than there is to run sage --make (which is not a thing, but offers the same "convenience").

[jdemeyer]

Replying to embray:

sage --make

That option is actually called sage -i :-)

In all seriousness: I really want to support a sage_patchbot installed within Sage itself.

You may argue that this is weird. But on the other hand: why not support it? You need good arguments to explicitly not support something. I never said that this should be the only way. I do not want to force everybody to use sage --patchbot. I am only saying: it costs little to support it, so why not?

[embray]

Personally I see no problem with supporting sage --patchbot. Just that it should not try to run a patchbot module that's installed outside the sage environment. In other words, it should run the patchbot that's installed in sage, or do nothing at all (or at most give an error).

[jdemeyer]

Replying to embray:

Personally I see no problem with supporting sage --patchbot. Just that it should not try to run a patchbot module that's installed outside the sage environment. In other words, it should run the patchbot that's installed in sage, or do nothing at all (or at most give an error).

That's fine for me... Frédéric, Vincent: what you do think?

[chapoton]

Whatever way this ticket is closed, I do not care. If you manage to agree on something, go on.

[jdemeyer]

So Frédéric, do you retract comment 85? Because then the easiest would be to go back to that branch (i.e. my branch from 76). Judging from Erik's comment, he should agree on that.

[chapoton]

go on

[embray]

Uhh, the branch at ​https://git.sagemath.org/sage.git/commit/?id=6d7990e88c9f194f28309bd0015c40ecc09080bf is what I'm objecting to. It attempts to run sage_patchbot with the system python as well, which I'm saying it shouldn't do. sage --whatever shouldn't touch the system Python except when building sage itself.

[jdemeyer]

So it seems that we cannot agree on anything then...

[embray]

I thought we did... please re-read ​https://trac.sagemath.org/ticket/20736#comment:110

Keep in mind when I write "run patchbot" I'm looking at the patchbot program as a black box. I don't care that it happens to be written in Python--nor should anyone else running it. So it's rather bizarre for sage --patchbot to try to do anything with a patchbot module that is not installed in the $SAGE_LOCAL or with any python that is not in $SAGE_LOCAL

I think I see the hold-up in my own thinking. There may not be a Python in $SAGE_LOCAL.

In that case I'm back to not understanding why you even need or want sage --patchbot

[embray]

Replying to jdemeyer:

You need good arguments to explicitly not support something.

I would say this is rarely true, and it is almost always the opposite that is true.

[jdemeyer]

Replying to embray:

I thought we did... please re-read ​https://trac.sagemath.org/ticket/20736#comment:110

Well, I want to run the pacthbot installed within Sage using the system Python. You seem to agree with the first part but not the second.

You cannot use the Python-in-Sage to run the patchbot because that one might be reinstalled: the patchbot builds Sage so it might rebuild the Python-in-Sage too. And reinstalling a program that you are running is a recipe for trouble.

[embray]

Okay, but when we say "the patchbot installed within Sage" you mean there's a copy of the sage_patchbot package in local/lib/python2.7/site-packages, right?

What is the advantage to doing that? site-packages is a special path as far as Python is concerned. A typical Python executable knows where to find its site-packages (well, it gets it from site.py which it knows how to find), and that site-packages in a sense "belongs" to that Python. So it's very atypical to use one Python installation to run a package that happens to be in another Python's site-packages. One exception is virtualenvs that make use of the "system site-packages" feature.

Not saying it won't work--of course ultimately it's just another directory full of Python modules one is adding to sys.path (assuming they are for the same Python version at least, which they might not be--your patch tries python2.7, followed by python2, followed by python, where the latter might be Python 2.6, or Python 3.4 or something).

TL;DR I don't see what you gain from that.

[mkoeppe]

I haven't followed the full discussion, nor have I ever run a patchbot, but here's my 2 cents.

+1 on using the system Python.

-1 on including the patchbot as part of the sage distribution in any way.

-1 on supporting sage --patchbot.

I think it's inviting trouble to have the testing framework depend in any way on scripts of the software to be tested.

[rws]

Replying to jdemeyer:

You cannot use the Python-in-Sage to run the patchbot because that one might be reinstalled: the patchbot builds Sage so it might rebuild the Python-in-Sage too. And reinstalling a program that you are running is a recipe for trouble.

Reinstalled by what, a Sage upgrade or a patchbot run?

[jdemeyer]

Replying to rws:

Replying to jdemeyer:

You cannot use the Python-in-Sage to run the patchbot because that one might be reinstalled: the patchbot builds Sage so it might rebuild the Python-in-Sage too. And reinstalling a program that you are running is a recipe for trouble.

Reinstalled by what, a Sage upgrade or a patchbot run?

A patchbot run (the patchbot obviously needs to build Sage, which might include a Python rebuild).

[jdemeyer]

Replying to embray:

TL;DR I don't see what you gain from that.

I am sure I have said this already multiple times: what I gain is not requiring a system-wide pip.

I just want the requirements to run the patchbot to be as minimal as possible. With my proposal, the only requirement is Sage.

[rws]

Replying to jdemeyer:

Replying to rws:

Replying to jdemeyer:

You cannot use the Python-in-Sage to run the patchbot because that one might be reinstalled: the patchbot builds Sage so it might rebuild the Python-in-Sage too. And reinstalling a program that you are running is a recipe for trouble.

Reinstalled by what, a Sage upgrade or a patchbot run?

A patchbot run (the patchbot obviously needs to build Sage, which might include a Python rebuild).

Just FYI: that doesn't apply to unsafe tickets (EDITED). Those get built in a sandbox under /tmp. But they don't work anyway atm.

[rws]

But wait, a Python rebuild happens potentially only with unsafe tickets, as well.

[rws]

So your precaution doesn't matter because Python rebuilds are done in /tmp, not in the Sage installation that runs the patchbot.

[jdemeyer]

Replying to rws:

But wait, a Python rebuild happens potentially only with unsafe tickets, as well.

Wrong. The patchbot also needs to build Sage in the usual way.

[jdemeyer]

By the way, I was unaware that "unsafe" tickets were never tested. I never realized that.

[rws]

You can test them but with some runs there were big problems. That's why it's discouraged to not use --safe-only.

[jdemeyer]

Replying to mkoeppe:

I think it's inviting trouble to have the testing framework depend in any way on scripts of the software to be tested.

My idea is that the patchbot would not depend on anything from Sage. Only that Sage would provide convenient (but optional) scripts to run the patchbot.

[klee]

This ticket seems stalled because jdemeyer insists to support running patchbot installed inside Sage by the system python using Sage interface, whose idea is objected by some others.

The main objective of the ticket is to make it official to install the patchbot only as a package to the system python. This objective is in direct conflict with the jdemeyer's idea.

So my suggestion is to separate the script implementing jdemeyer's idea from the present ticket, and he opens a ticket for his script. This will facilitate closing this ticket.

[jdemeyer]

Replying to klee:

This objective is in direct conflict with the jdemeyer's idea.

Absolutely not, there is no conflict! I just want one additional way to run the patchbot which does not harm other ways of running the patchbot.

I don't see why people are so much against an additional feature. I'm not forcing anybody to run the patchbot "my way", I just want to keep the possibility.

[jakobkroeker]

--sage-root=

1. the parameter should be called --sage-testing-root= (or similar) rather than sage-root: several sage installations may exists on a system. Especially sage-root might be menthally associated with the production-sage root directory.

2. The option sage-testing-root should be added as a config file parameter.

If supporting 'sage -patchbot' works as expected (starts the user-local or system installation of the patchbot and also uses the system python) and does not cause any serious harm (including maintenance issues) I would not insist on the complete removal of 'sage -patchbot'. A compromise between the two different opinions here could be to print a message that running 'sage -patchbot' is no longer the preferred way to run the patchbot and point to the buildbot wiki.

sloppy script

what is exactly the issue of the script by Jeroen?

[jakobkroeker]

The option sage-root should be added as a config file parameter.

@chapoton

I just discovered that it is, but in the config file the parameter name has to be sage_root (underscore instead of a dash), so here is an inconsistency. Also please update the wiki instructions https://wiki.sagemath.org/buildbot/details accordingly.

[jdemeyer]

IMHO, it's obvious that the --sage-root when running the patchbot is the Sage installation that you want to test. I don't see the point of using a more obscure name like --sage-testing-root for the option.

[jakobkroeker]

Replying to jdemeyer:

is the Sage installation that you want to test.

If I want to test a sage installation I just type './sage -t --all '

Of course you mean that 'sage-root' is here the folder where patchbot will test various tickets (and thus various sage versions)

IMHO, it's obvious that the --sage-root when running the patchbot is the Sage installation that you want to test.

Since you formally failed to correcly explain what --sage-root means, I do not buy that 'it is obvious', at least not for a new patchbot user. So I find the 'sage-root' patchbot option should be at lieast documented (what it does) on the wiki and in the software and the inconsistency between 'sage_root' and 'sage-root' should be removed (underscore vs dash)

By the way; it is well known that naming is one important part of software development and it may create mental blockades or misunderstandings.

In summary: I expect as minimal action to gain positive review that 'sage_root' gets documented in the software and in the wiki (​https://wiki.sagemath.org/buildbot/details) and the inconsistency between 'sage_root' and 'sage-root' is removed (underscore in config file vs dash in command line option)

[jakobkroeker]

And the 'sage -patchbot' command should print a warning that 'sage -patchbot' is no longer the preferred way to use patchbot and print the preferred way to use the patchbot or link to the wiki (​https://wiki.sagemath.org/buildbot/details)

[embray]

Replying to jdemeyer:

IMHO, it's obvious that the --sage-root when running the patchbot is the Sage installation that you want to test. I don't see the point of using a more obscure name like --sage-testing-root for the option.

Here we agree--the purpose of patchbot is testing sage installations. --sage-root is the path to the sage installation you want it to test. If there is some documentation that needs to be improved that's one thing, but putting "testing" in the option name is superfluous.

As for sage -patchbot, no there's no conflict. I'm -0.5 on it--I think it does no harm, ultimately, but it also comes across as code rot to me and doesn't serve any purpose I can glean except for "backwards compatibility", but with what I think is ultimately a misfeature anyways :/

[jdemeyer]

Replying to embray:

doesn't serve any purpose

Well, if it didn't serve any purpose, I wouldn't insist so much. See 54 and following.

[embray]

I'm not sure why you need to run python -m sage_patchbot.patchbot. Although that's not bad practice in cases where you want to make sure you're running the right sage_patchbot with the right python, in most cases (I think?) it suffices to just run patchbot.

So ISTM the main purpose of ./sage -patchbot is to provide a sane default for the --sage-root option. First of all, this already gets set from the SAGE_ROOT environment variable if it is set. But what if the pwd is also a valid SAGE_ROOT? Then it could take the pwd to be the default (if SAGE_ROOT is not set). That would incorporate all the convenience of running ./sage -patchbot from the same directory--more even. Compare:

$ cd my_sage
$ ./sage -patchbot

$ cd my_sage
$ patchbot

[git]

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

​e75e8de	Update patchbot to 2.6.1 as system package
​d7c83f1	Improve --patchbot command

[jdemeyer]

Rebased to Sage-7.5.rc1

[tmonteil]

Since this ticket seems not to reach a consensus, what about splitting it into two part:

• remove patchbot optional spkg, and deprecate the related sage -patchbot command (this can be done for next beta)
• discuss and write a convenient alternative command or documentation to run the patchbot (this seems the blocking point and might require more time)

I can volonteer to implement the first point on top of the latest beta ;)

[jdemeyer]

Replying to tmonteil:

• remove patchbot optional spkg, and deprecate the related sage -patchbot command (this can be done for next beta)
• discuss and write a convenient alternative command or documentation to run the patchbot (this seems the blocking point and might require more time)

Doesn't "deprecate the related sage -patchbot command" relate more to the second point instead of the first?

So I would rather split up as

1. remove patchbot optional spkg
2. discuss how to run the patchbot and what to do with sage --patchbot

[tmonteil]

OK, see #22473.

Replying to jdemeyer:

Doesn't "deprecate the related sage -patchbot command" relate more to the second point instead of the first?

So I would rather split up as

1. remove patchbot optional spkg
2. discuss how to run the patchbot and what to do with sage --patchbot

OK, I did not deprecate the command, but since the current sage -patchbot command does not work anyway, i replaced it with a short sentence pointing to ​https://wiki.sagemath.org/buildbot#Installing_the_patchbot (which are the most up-to-date instructions). It is just a suggestion until the current ticket get fixed, i can undo the last commit about the sage -patchbot command if required.

[jdemeyer]

Replying to tmonteil:

It is just a suggestion until the current ticket get fixed

For the record: in my opinion, nothing needs to be fixed since nothing is broken.

[jdemeyer]

Replying to tmonteil:

​https://wiki.sagemath.org/buildbot#Installing_the_patchbot (which are the most up-to-date instructions). It is just a suggestion until the current ticket get fixed

It's also an invitation to move the current discussion on this ticket to an edit war on that wiki page, so I am not so happy with that.

[tmonteil]

Replying to jdemeyer:

Replying to tmonteil:

​https://wiki.sagemath.org/buildbot#Installing_the_patchbot (which are the most up-to-date instructions). It is just a suggestion until the current ticket get fixed

It's also an invitation to move the current discussion on this ticket to an edit war on that wiki page, so I am not so happy with that.

OK, i removed that commit, the goal is to get rid of this broken optional package earlier, without entering the picky part of the current ticket.

[embray]

I think IIRC I was fine with keeping sage -patchbot as a shortcut for "run patchbot with the --sage-root option set from this sage). I think that makes sense, and I think that's what Jeroen was trying to do originally. I just thought the solution was overcomplicated.

I don't know why we're talking about python -m anything. When you install the sage-patchbot Python package (which Frédéric did a great job putting together at my urging) it installs a patchbot script. If you run that it just works, you just have to supply the correct --sage-root argument.

[jdemeyer]

What I want in particular is to install the patchbot within the Sage environment and have sage --patchbot recognize that.

Of course, I am totally open to supporting any other use cases too (unlike some other people who explicitly do not want to suppport my use case).

[embray]

Was there some reason that wouldn't work? That is, if you ran sage -pip install sage-patchbot? Then, in principle, it would work if you ran sage -sh -c patchbot though I guess if $SAGE_LOCAL/bin/patchbot exists then sage -patchbot can be a shortcut for that. I'm just not sure why you'd need or want to do that but I don't oppose it.

[jdemeyer]

Replying to embray:

I don't know why we're talking about python -m anything. When you install the sage-patchbot Python package (which Frédéric did a great job putting together at my urging) it installs a patchbot script.

...in a directory which might not be the default $PATH (especially in the case of a --user install). So there are cases where python -m sage_patchbot.patchbot works but running patchbot does not.

[git]

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

​b2810d6	#22473 : remove the patchbot optional spkg.
​537849f	trac 20736 first try, failing
​b423b62	Improve --patchbot command

[jdemeyer]

Rebased to #22473.

[git]

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

​c9c4101

Improve --patchbot command

[jdemeyer]

Let's start over...

[embray]

Replying to jdemeyer:

Replying to embray:

I don't know why we're talking about python -m anything. When you install the sage-patchbot Python package (which Frédéric did a great job putting together at my urging) it installs a patchbot script.

...in a directory which might not be the default $PATH (especially in the case of a --user install). So there are cases where python -m sage_patchbot.patchbot works but running patchbot does not.

That would imply someone has their ~/.local/lib/pythonX.Y/site-packages on sys.path, but not ~/.local/bin in their $PATH. That can certainly happen (I think it is the default in most cases) but I would consider it a slight misconfiguration. It is also not a problem at all if using virtualenvs instead.

[jdemeyer]

I wonder what made you change your mind...

This code (which has been there since 101) has been attacked by several people for 6 months and now it suddenly has positive_review?

[embray]

My feeling at this point is along the lines of "I'm not going to use it, but if it suits your needs then fine." :)