Opened 5 years ago
Closed 4 years ago
#20736 closed enhancement (fixed)
Upgrade patchbot to 2.6.1 as a system package
Reported by: | chapoton | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.6 |
Component: | packages: optional | Keywords: | patchbot |
Cc: | embray, jdemeyer | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | c9c4101 (Commits) | Commit: | c9c4101b961c454b2acac23508a74ccfbcffb7c4 |
Dependencies: | #22473 | Stopgaps: |
Description (last modified by )
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):
- Download the tarball and then
pip install --user sage-patchbot-2.6.1.tar.gz
- (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
- Checkout the branch here and run
make && ./sage --patchbot
Change History (163)
comment:1 Changed 5 years ago by
- Description modified (diff)
comment:2 Changed 5 years ago by
- Branch set to public/20736
- Commit set to a827087d9b4335f351605f2d2a119b53d0c3bd25
comment:3 Changed 5 years ago by
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.
comment:4 Changed 5 years ago by
- Commit changed from a827087d9b4335f351605f2d2a119b53d0c3bd25 to c9907efed9c7f02a928af40a2514a28f2ed7e356
Branch pushed to git repo; I updated commit sha1. New commits:
c9907ef | trac 20736 change in dependencies
|
comment:5 Changed 5 years ago by
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)
comment:6 Changed 5 years ago by
- Description modified (diff)
comment:7 Changed 5 years ago by
- Commit changed from c9907efed9c7f02a928af40a2514a28f2ed7e356 to f3839fd6c0fb2562fb7731e0a62f73a2d3b89124
Branch pushed to git repo; I updated commit sha1. New commits:
f3839fd | another try, failing the same way
|
comment:8 Changed 5 years ago by
I am making some slow progress.
comment:9 Changed 5 years ago by
- Description modified (diff)
comment:10 Changed 5 years ago by
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.
comment:11 Changed 5 years ago by
- Commit changed from f3839fd6c0fb2562fb7731e0a62f73a2d3b89124 to 160332c495872a01047d109a8584aca955d98252
Branch pushed to git repo; I updated commit sha1. New commits:
160332c | fixing the spkg-install script
|
comment:12 follow-up: ↓ 13 Changed 5 years ago by
So it's a build dep for Sage and should be installed "the usual way" using the system Python.
comment:13 in reply to: ↑ 12 ; follow-up: ↓ 15 Changed 5 years ago by
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.
comment:14 follow-up: ↓ 16 Changed 5 years ago by
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.
comment:15 in reply to: ↑ 13 Changed 5 years ago by
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.
comment:16 in reply to: ↑ 14 Changed 5 years ago by
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.
comment:17 Changed 5 years ago by
- Commit changed from 160332c495872a01047d109a8584aca955d98252 to 228842b1aad446925645dcadb72f9c768358fe0d
- Description modified (diff)
- Summary changed from make patchbot a correct python package to upgrade patchbot to 2.6.0 (compatible with https trac)
comment:18 Changed 5 years ago by
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 ??
comment:19 Changed 5 years ago by
- Description modified (diff)
comment:20 Changed 5 years ago by
- Description modified (diff)
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)
comment:21 Changed 5 years ago by
- Description modified (diff)
comment:22 in reply to: ↑ description ; follow-up: ↓ 34 Changed 5 years ago by
- Commit changed from 228842b1aad446925645dcadb72f9c768358fe0d to 91fc4927d5115ffd9af20b45b9cc5d05a4b284ef
comment:23 Changed 5 years ago by
Can you please update the patchbot to only deal with the https thing and not change the whole install procedure?
comment:24 follow-up: ↓ 25 Changed 5 years ago by
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.
comment:25 in reply to: ↑ 24 ; follow-up: ↓ 26 Changed 5 years ago by
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.
comment:26 in reply to: ↑ 25 ; follow-ups: ↓ 28 ↓ 29 Changed 5 years ago by
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 ?
comment:27 Changed 5 years ago by
- Description modified (diff)
- Summary changed from upgrade patchbot to 2.6.0 (compatible with https trac) to Upgrade patchbot to 2.6.0 to system package
comment:28 in reply to: ↑ 26 Changed 5 years ago by
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.
comment:29 in reply to: ↑ 26 Changed 5 years ago by
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.
comment:30 follow-up: ↓ 31 Changed 5 years ago by
If the patchbot
becomes a system package, it should be removed as Sage package. I.e. remove the whole directory build/pkgs/patchbot
.
comment:31 in reply to: ↑ 30 ; follow-up: ↓ 32 Changed 5 years ago by
Replying to jdemeyer:
If the
patchbot
becomes a system package, it should be removed as Sage package. I.e. remove the whole directorybuild/pkgs/patchbot
.
yes, I agree.
And there is an __init__.py
in the package.
comment:32 in reply to: ↑ 31 Changed 5 years ago by
- Commit changed from 91fc4927d5115ffd9af20b45b9cc5d05a4b284ef to 897f5b220c7eb63a994853092be376d62e40fe39
comment:33 Changed 5 years ago by
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.
comment:34 in reply to: ↑ 22 Changed 5 years ago by
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.
comment:35 follow-ups: ↓ 36 ↓ 37 Changed 5 years ago by
- Status changed from new to needs_review
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.
comment:36 in reply to: ↑ 35 Changed 5 years ago by
Replying to chapoton:
On the other hand, arando is uselessly running 2.5.7.
I know, I have not upgraded it yet.
comment:37 in reply to: ↑ 35 Changed 5 years ago by
- Reviewers set to Jeroen Demeyer
- Status changed from needs_review to needs_work
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.
comment:38 follow-up: ↓ 54 Changed 5 years ago by
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?
comment:39 Changed 5 years ago by
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.
comment:40 Changed 5 years ago by
much more annoying is the fact that the patchbot persist to test closed tickets..
comment:41 Changed 5 years ago by
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.
comment:42 Changed 5 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
- Summary changed from Upgrade patchbot to 2.6.0 to system package to Upgrade patchbot to 2.6.1 as a system package
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 ?
comment:43 follow-up: ↓ 48 Changed 5 years ago by
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'
comment:44 follow-up: ↓ 49 Changed 5 years ago by
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, ...
comment:45 Changed 5 years ago by
(I launch the package version of the patchbot on librae)
comment:46 Changed 5 years ago by
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?
comment:47 Changed 5 years ago by
What about switching to logging Python module?
comment:48 in reply to: ↑ 43 ; follow-ups: ↓ 50 ↓ 51 Changed 5 years ago by
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.
comment:49 in reply to: ↑ 44 Changed 5 years ago by
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.
comment:50 in reply to: ↑ 48 Changed 5 years ago by
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.
comment:51 in reply to: ↑ 48 Changed 5 years ago by
Replying to chapoton:
Replying to vdelecroix:
If I install the package on my system Python, then doing
$ python -m sage_patchbot.patchbotThanks 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.
comment:52 Changed 5 years ago by
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
comment:53 Changed 5 years ago by
Maybe due to swap (I need to check)...
comment:54 in reply to: ↑ 38 Changed 5 years ago by
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.
comment:55 Changed 5 years ago by
- Status changed from needs_review to needs_info
Please update the ticket description with the recommended way to install and run the patchbot. Then I will test it.
comment:56 Changed 5 years ago by
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).
comment:57 Changed 4 years ago by
- Description modified (diff)
@jdemeyer
I have tried to enhance the description of the ticket.
I am not willing to avoid the dependency to pip.
comment:58 Changed 4 years ago by
- Description modified (diff)
Like I said before, ./sage --patchbot
does not actually work.
comment:59 Changed 4 years ago by
See the last line of 37
comment:60 Changed 4 years ago by
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).
comment:61 Changed 4 years ago by
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.
comment:62 Changed 4 years ago by
- Commit changed from 897f5b220c7eb63a994853092be376d62e40fe39 to 14fb5998257fe1f7bc352d92c0de9571ae85a11d
comment:63 Changed 4 years ago by
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.
comment:64 Changed 4 years ago by
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.
comment:65 Changed 4 years ago by
To turn the question around: why do you keep insisting on not supporting a patchbot installed within Sage?
comment:66 follow-up: ↓ 71 Changed 4 years ago by
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.
comment:67 follow-up: ↓ 69 Changed 4 years ago by
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
comment:68 Changed 4 years ago by
Jeroen, do you also prefer (1) ?
comment:69 in reply to: ↑ 67 ; follow-up: ↓ 70 Changed 4 years ago by
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.
comment:70 in reply to: ↑ 69 ; follow-up: ↓ 74 Changed 4 years ago by
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_rootNo, 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.
comment:71 in reply to: ↑ 66 Changed 4 years ago by
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.
comment:72 Changed 4 years ago by
- Commit changed from 14fb5998257fe1f7bc352d92c0de9571ae85a11d to 654e5788ec2e32f06ef246f09ade24dc6ddf20bb
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
654e578 | Improve --patchbot command
|
comment:73 Changed 4 years ago by
First attempt to support a patchbot installed within Sage, not fully tested yet.
comment:74 in reply to: ↑ 70 Changed 4 years ago by
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.
comment:75 Changed 4 years ago by
Hang on, I have a better solution in mind to run the patchbot.
comment:76 Changed 4 years ago by
- Commit changed from 654e5788ec2e32f06ef246f09ade24dc6ddf20bb to 6d7990e88c9f194f28309bd0015c40ecc09080bf
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
6d7990e | Improve --patchbot command
|
comment:77 Changed 4 years ago by
- Status changed from needs_info to needs_review
Currently testing...
comment:78 Changed 4 years ago by
This is good for me. Can anybody review my commit?
comment:79 Changed 4 years ago by
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
comment:80 follow-up: ↓ 81 Changed 4 years ago by
Instead of a sloppy script, I would be very much happier with sage -patchbot
returning an informative error message.
comment:81 in reply to: ↑ 80 Changed 4 years ago by
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.
comment:82 follow-up: ↓ 86 Changed 4 years ago by
So what exactly is necessary to make this branch work, i.e. to run sage -patchbot
successfully?
comment:83 Changed 4 years ago by
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.
comment:84 Changed 4 years ago by
Please do not interfere with the wiki instructions until this ticket is closed. Revert your changes there, please.
comment:85 Changed 4 years ago by
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
comment:86 in reply to: ↑ 82 Changed 4 years ago by
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.
comment:87 Changed 4 years ago by
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.
comment:88 Changed 4 years ago by
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".
comment:89 Changed 4 years ago by
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.
comment:90 follow-up: ↓ 91 Changed 4 years ago by
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.
comment:91 in reply to: ↑ 90 Changed 4 years ago by
Replying to chapoton:
And this is still only two command lines.
Actually, it's five commands:
- Download
pip
- Install
pip
- Download
sage_patchbot
- Install
sage_patchbot
- Run
sage_patchbot
comment:92 Changed 4 years ago by
Meaning no offense, this comment is slightly small-minded, IMHO
comment:93 follow-up: ↓ 94 Changed 4 years ago by
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
comment:94 in reply to: ↑ 93 ; follow-up: ↓ 95 Changed 4 years ago by
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 -patchbotover
$ patchbot
The fact that ./sage --patchbot
does not require the user to change his $PATH
.
comment:95 in reply to: ↑ 94 Changed 4 years ago by
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 -patchbotover
$ patchbotThe fact that
./sage --patchbot
does not require the user to change his$PATH
.
Why would a user need to change their path?
comment:96 Changed 4 years ago by
- Milestone changed from sage-7.3 to sage-7.4
- Status changed from needs_review to needs_work
comment:97 Changed 4 years ago by
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
comment:98 follow-up: ↓ 99 Changed 4 years ago by
There is something very wrong in how the patchbot gets its tickets from the server...
comment:99 in reply to: ↑ 98 ; follow-up: ↓ 100 Changed 4 years ago by
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.
comment:100 in reply to: ↑ 99 Changed 4 years ago by
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.
comment:101 Changed 4 years ago by
- Commit changed from 6d7990e88c9f194f28309bd0015c40ecc09080bf to fc8bcec8a2d753fcf1a9eaf451258081cf02726b
comment:102 Changed 4 years ago by
- Dependencies set to #21430
- Status changed from needs_work to needs_review
Rebased over #21430, partially squashed, fixed installations in system Python.
comment:103 Changed 4 years ago by
I am currently running the patchbot on sage4
and arando
with this branch.
comment:104 follow-up: ↓ 105 Changed 4 years ago by
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?
comment:105 in reply to: ↑ 104 ; follow-up: ↓ 106 Changed 4 years ago by
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.
comment:106 in reply to: ↑ 105 ; follow-ups: ↓ 107 ↓ 108 Changed 4 years ago by
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.
comment:107 in reply to: ↑ 106 Changed 4 years ago by
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).
comment:108 in reply to: ↑ 106 ; follow-up: ↓ 109 Changed 4 years ago by
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").
comment:109 in reply to: ↑ 108 ; follow-up: ↓ 118 Changed 4 years ago by
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?
comment:110 follow-up: ↓ 111 Changed 4 years ago by
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).
comment:111 in reply to: ↑ 110 Changed 4 years ago by
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?
comment:112 Changed 4 years ago by
Whatever way this ticket is closed, I do not care. If you manage to agree on something, go on.
comment:113 Changed 4 years ago by
comment:114 Changed 4 years ago by
go on
comment:115 Changed 4 years ago by
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.
comment:116 Changed 4 years ago by
So it seems that we cannot agree on anything then...
comment:117 follow-up: ↓ 119 Changed 4 years ago by
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
comment:118 in reply to: ↑ 109 Changed 4 years ago by
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.
comment:119 in reply to: ↑ 117 ; follow-up: ↓ 122 Changed 4 years ago by
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.
comment:120 follow-up: ↓ 124 Changed 4 years ago by
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.
comment:121 follow-up: ↓ 131 Changed 4 years ago by
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.
comment:122 in reply to: ↑ 119 ; follow-up: ↓ 123 Changed 4 years ago by
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?
comment:123 in reply to: ↑ 122 ; follow-up: ↓ 125 Changed 4 years ago by
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).
comment:124 in reply to: ↑ 120 Changed 4 years ago by
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.
comment:125 in reply to: ↑ 123 Changed 4 years ago by
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.
comment:126 follow-up: ↓ 128 Changed 4 years ago by
But wait, a Python rebuild happens potentially only with unsafe tickets, as well.
comment:127 Changed 4 years ago by
So your precaution doesn't matter because Python rebuilds are done in /tmp, not in the Sage installation that runs the patchbot.
comment:128 in reply to: ↑ 126 Changed 4 years ago by
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.
comment:129 Changed 4 years ago by
By the way, I was unaware that "unsafe" tickets were never tested. I never realized that.
comment:130 Changed 4 years ago by
You can test them but with some runs there were big problems. That's why it's discouraged to not use --safe-only
.
comment:131 in reply to: ↑ 121 Changed 4 years ago by
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.
comment:132 follow-up: ↓ 133 Changed 4 years ago by
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.
comment:133 in reply to: ↑ 132 Changed 4 years ago by
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.
comment:134 Changed 4 years ago by
- Status changed from needs_review to needs_work
--sage-root=
- the parameter should be called
--sage-testing-root=
(or similar) rather thansage-root
: several sage installations may exists on a system. Especially sage-root might be menthally associated with the production-sage root directory.
- 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?
comment:135 Changed 4 years ago by
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.
comment:136 follow-ups: ↓ 137 ↓ 139 Changed 4 years ago by
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.
comment:137 in reply to: ↑ 136 Changed 4 years ago by
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)
comment:138 Changed 4 years ago by
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)
comment:139 in reply to: ↑ 136 ; follow-up: ↓ 140 Changed 4 years ago by
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 :/
comment:140 in reply to: ↑ 139 Changed 4 years ago by
comment:141 Changed 4 years ago by
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
comment:142 Changed 4 years ago by
- Commit changed from fc8bcec8a2d753fcf1a9eaf451258081cf02726b to d7c83f1b06d1d851ef012acdf99a105cbe9db3ac
comment:143 Changed 4 years ago by
- Dependencies #21430 deleted
- Milestone changed from sage-7.4 to sage-7.6
Rebased to Sage-7.5.rc1
comment:144 follow-up: ↓ 145 Changed 4 years ago by
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 ;)
comment:145 in reply to: ↑ 144 ; follow-up: ↓ 146 Changed 4 years ago by
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
- remove patchbot optional spkg
- discuss how to run the patchbot and what to do with
sage --patchbot
comment:146 in reply to: ↑ 145 ; follow-ups: ↓ 147 ↓ 148 Changed 4 years ago by
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
- remove patchbot optional spkg
- 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.
comment:147 in reply to: ↑ 146 Changed 4 years ago by
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.
comment:148 in reply to: ↑ 146 ; follow-up: ↓ 149 Changed 4 years ago by
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.
comment:149 in reply to: ↑ 148 Changed 4 years ago by
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.
comment:150 follow-up: ↓ 154 Changed 4 years ago by
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.
comment:151 Changed 4 years ago by
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).
comment:152 Changed 4 years ago by
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.
comment:153 Changed 4 years ago by
- Dependencies set to #22473
comment:154 in reply to: ↑ 150 ; follow-up: ↓ 159 Changed 4 years ago by
Replying to embray:
I don't know why we're talking about
python -m
anything. When you install thesage-patchbot
Python package (which Frédéric did a great job putting together at my urging) it installs apatchbot
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.
comment:155 Changed 4 years ago by
- Commit changed from d7c83f1b06d1d851ef012acdf99a105cbe9db3ac to b423b62329b9d028a8a6de11def5fbe1eae24519
comment:156 Changed 4 years ago by
Rebased to #22473.
comment:157 Changed 4 years ago by
- Commit changed from b423b62329b9d028a8a6de11def5fbe1eae24519 to c9c4101b961c454b2acac23508a74ccfbcffb7c4
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c9c4101 | Improve --patchbot command
|
comment:158 Changed 4 years ago by
- Status changed from needs_work to needs_review
Let's start over...
comment:159 in reply to: ↑ 154 Changed 4 years ago by
Replying to jdemeyer:
Replying to embray:
I don't know why we're talking about
python -m
anything. When you install thesage-patchbot
Python package (which Frédéric did a great job putting together at my urging) it installs apatchbot
script....in a directory which might not be the default
$PATH
(especially in the case of a--user
install). So there are cases wherepython -m sage_patchbot.patchbot
works but runningpatchbot
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.
comment:160 Changed 4 years ago by
- Status changed from needs_review to positive_review
comment:161 Changed 4 years ago by
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?
comment:162 Changed 4 years ago by
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." :)
comment:163 Changed 4 years ago by
- Branch changed from public/20736 to c9c4101b961c454b2acac23508a74ccfbcffb7c4
- Resolution set to fixed
- Status changed from positive_review to closed
I tried and failed so far. Help is required, please.
New commits:
trac 20736 first try, failing