Make the Sage patchbot an optional spkg

[robertwb]

Anyone can run sage --patchbot to contribute.

Instructions:

• apply attachment:patchbot-root-5.0.quoted.patch​ to the SAGE_ROOT repo and
• install ​http://sage.math.washington.edu/home/robertwb/patches/patchbot-1.0.spkg

[ddrake]

note that this doesn't apply to 5.0.beta3, since the sage-sage script has been deprecated. It looks like the relevant bit needs to be applied to SAGE_ROOT/spkg/bin/sage for 5.0.

[ddrake]

I split the original patch, since in 5.0, the sage script is tracked by the root repo.

[ddrake]

We should document the patchbot. Something better than "read the code" and "ask Robert on sage-devel". This is now #12505.

[ddrake]

for 5.0 series, apply to SAGE_ROOT repo

[ddrake]

New patches (1) remove trailing whitespace, (2) include ticket number in commit message, and (3) have proper node and parent IDs, so patchbot knows where to apply them. These changes were prompted by looking at the patchbot's logs for this ticket. :)

[davidloeffler]

I tried this out (you'll see there are some results from debian/squeeze/sid/x86_64/2.6.32-37-server/fermat on the master patchbot server) and I got some strange behaviour. If you run the bot and give it a ticket number, with the --ticket command-line option, then it tests that ticket, but on finishing the test it gets stuck in an infinite loop of idling roundabout line 178 of patchbot.py (in particular, it never sends the test results back to the server). This is kind of annoying.

[davidloeffler]

Sorry, the issue I reported the last time is a non-issue (you just have to wait a bit longer for the Tee background process to notice that it's finished).

But there is an amusing bug which I spotted when running a patchbot under 5.0.beta10. The function that compares version numbers just splits on "." and sorts alphabetically, and hence it thinks beta1 < beta10 < beta2! This caused a bunch of spurious "Apply Failed" warnings when it tried to apply patches from tickets that had already been merged (listed as dependencies of open tickets). See for example the first of the two runs for ticket #10527.

The fix is to add the lines

a = a.replace("beta", "beta.")
b = b.replace("beta", "beta.")

to util.compare_versions.

[ddrake]

for 5 .0 series, apply to scripts repo

[ddrake]

Replying to davidloeffler:

The fix is to add the lines

a = a.replace("beta", "beta.")
b = b.replace("beta", "beta.")

to util.compare_versions.

The updated patch does that. (It also is a bit more strict about catching a ValueError when doing int(s), which probably isn't a big deal, but it seems like a good idea.)

[robertwb]

Thanks. I noticed that too.

[jdemeyer]

Since a while now, there is no requirement for the ticket number to be in the commit message, it's added automatically when merging in the standard format

Trac #12486: message from the patch file

[kini]

I noticed just now that on the buildbot arando, where I am running a patchbot on Sage 5.1.beta1, there were orphaned patchbot testing processes (i.e. instances of the python interpreter) left lying around, running things in ~/.sage/tmp/arando-*/, of which many seemed to be named maxima_abstract_*.py, though there were several other filenames as well. These testing processes were not sleeping or waiting but running full steam, taking up most of arando's CPU time, so I killed them. I've noticed this happen once or twice before, as well.

[jdemeyer]

kini: I'd love to know which ticket causes the maxima_abstract failures.

[jdemeyer]

By the way: the "Python keeps running after a doctest timeout" bug is #11338.

[kini]

[2] patchbot@arando ~/.sage/tmp $ find | grep maxima_abstract | xargs grep "^#.*devel/sage-"
./arando-13446/maxima_abstract_22717.py:# '/opt/patchbot-5.1.beta0/devel/sage-12214/sage/interfaces/maxima_abstract.py'.
./arando-698/maxima_abstract_10001.py:# '/opt/patchbot-5.1.beta0/devel/sage-12120/sage/interfaces/maxima_abstract.py'.
./arando-21967/maxima_abstract_10092.py:# '/opt/patchbot-5.1.beta0/devel/sage-12989/sage/interfaces/maxima_abstract.py'.
./arando-25899/maxima_abstract_2742.py:# '/opt/patchbot-5.1.beta0/devel/sage-12298/sage/interfaces/maxima_abstract.py'.
./arando-25577/maxima_abstract_13608.py:# '/opt/patchbot-5.1.beta0/devel/sage-6812/sage/interfaces/maxima_abstract.py'.
./arando-7379/maxima_abstract_27734.py:# '/opt/patchbot-5.1.beta0/devel/sage-6812/sage/interfaces/maxima_abstract.py'.
./arando-16948/maxima_abstract_3744.py:# '/opt/patchbot-5.1.beta1/devel/sage-12043/sage/interfaces/maxima_abstract.py'.
./arando-5871/maxima_abstract_15144.py:# '/opt/patchbot-5.1.beta0/devel/sage-11726/sage/interfaces/maxima_abstract.py'.
./arando-10252/maxima_abstract_30489.py:# '/opt/patchbot-5.1.beta0/devel/sage-12518/sage/interfaces/maxima_abstract.py'.
./arando-25814/maxima_abstract_13608.py:# '/opt/patchbot-5.1.beta0/devel/sage-6812/sage/interfaces/maxima_abstract.py'.
./arando-21669/maxima_abstract_30915.py:# '/opt/patchbot-5.0/devel/sage-2215/sage/interfaces/maxima_abstract.py'.
./arando-3379/maxima_abstract_22614.py:# '/opt/patchbot-5.1.beta1/devel/sage-12251/sage/interfaces/maxima_abstract.py'.
./arando-23007/maxima_abstract_11131.py:# '/opt/patchbot-5.1.beta0/devel/sage-12989/sage/interfaces/maxima_abstract.py'.
./arando-7740/maxima_abstract_11922.py:# '/opt/patchbot-5.1.beta1/devel/sage-11930/sage/interfaces/maxima_abstract.py'.
./arando-13364/maxima_abstract_22646.py:# '/opt/patchbot-5.1.beta0/devel/sage-11143/sage/interfaces/maxima_abstract.py'.
./arando-12724/maxima_abstract_834.py:# '/opt/patchbot-5.1.beta0/devel/sage-12892/sage/interfaces/maxima_abstract.py'.
./arando-20437/maxima_abstract_29714.py:# '/opt/patchbot-5.1.beta0/devel/sage-11895/sage/interfaces/maxima_abstract.py'.
./arando-5929/maxima_abstract_15208.py:# '/opt/patchbot-5.1.beta0/devel/sage-12352/sage/interfaces/maxima_abstract.py'.
./arando-4951/maxima_abstract_14224.py:# '/opt/patchbot-5.1.beta0/devel/sage-2215/sage/interfaces/maxima_abstract.py'.

I don't know how many of these are files that these stalled processes were running and how many are just other temp files that happened not to have been cleaned up - next time this happens I'll grep ps instead. (The ticket numbers are in the path names of the form *devel/sage-*, not of the form ./arando-*.)

[jdemeyer]

There is no logic behind this, too many independent tickets have this issue.

[jdemeyer]

Is there a way how "state" could be preserved across patchbot runs? Could it happen that the patchbot tests ticket X and somehow ticket X has a bug causing failures in following tickets the patchbot tests? Because it looks like this is happening here.

I did 600 test runs of maxima_abstract.py on vanilla sage-5.1.beta2 and found no error. Nobody has reported this bug on the mailing list and I haven't seen it on the buildbot with sage-5.1.beta2 or earlier. So I conclude that the bug is introduced by a not-yet-merged ticket.

Since it is extremely unlikely that all the tickets in kini's list above cause the same bug, the most logical explanation is that somehow one ticket corrupted further patchbot runs.

[jdemeyer]

For the record, here is the verbose output of the failure:

sage -t --long --verbose "devel/sage/sage/interfaces/maxima_abstract.py"
[...]
Trying:
    ~f###line 2304:_sage_    >>> ~f
Expecting:
    1/sin(x)
**********************************************************************
File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/devel/sage/sage/interfaces/maxima_abstract.py", line ?, in __main__.example_81
Failed example:
    ~f###line 2304:_sage_    >>> ~f
Exception raised:
    Traceback (most recent call last):
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_81[3]>", line 1, in <module>
        ~f###line 2304:_sage_    >>> ~f
      File "element.pyx", line 1833, in sage.structure.element.RingElement.__invert__ (sage/structure/element.c:13904)
      File "element.pyx", line 1799, in sage.structure.element.RingElement.__div__ (sage/structure/element.c:13362)
      File "coerce.pyx", line 744, in sage.structure.coerce.CoercionModel_cache_maps.bin_op (sage/structure/coerce.c:6704)
      File "coerce.pyx", line 858, in sage.structure.coerce.CoercionModel_cache_maps.canonical_coercion (sage/structure/coerce.c:7892)
      File "map.pyx", line 1282, in sage.categories.map.FormalCompositeMap._call_ (sage/categories/map.c:6190)
      File "morphism.pyx", line 159, in sage.categories.morphism.CallMorphism._call_ (sage/categories/morphism.c:3080)
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python/site-packages/sage/interfaces/interface.py", line 200, in __cal
l__
        return self._coerce_from_special_method(x)
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python/site-packages/sage/interfaces/interface.py", line 226, in _coer
ce_from_special_method
        return (x.__getattribute__(s))(self)
      File "sage_object.pyx", line 527, in sage.structure.sage_object.SageObject._maxima_ (sage/structure/sage_object.c:5444)
        This function may call the magma interpreter when it runs.
      File "sage_object.pyx", line 439, in sage.structure.sage_object.SageObject._interface_ (sage/structure/sage_object.c:3684)
        X = I(s)
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python/site-packages/sage/interfaces/interface.py", line 198, in __call__
        return cls(self, x, name=name)
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python/site-packages/sage/interfaces/maxima.py", line 1155, in __init__
        ExpectElement.__init__(self, parent, value, is_name=False, name=None)
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python/site-packages/sage/interfaces/expect.py", line 1331, in __init__
        self._name = parent._create(value, name=name)
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python/site-packages/sage/interfaces/interface.py", line 388, in _create
        self.set(name, value)
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python/site-packages/sage/interfaces/maxima.py", line 1000, in set
        self._eval_line(cmd)
      File "/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python/site-packages/sage/interfaces/maxima.py", line 756, in _eval_line
        assert line_echo.strip() == line.strip()
    AssertionError
[...]
4 items had no tests:
    __main__
    __main__.change_warning_output
    __main__.check_with_tolerance
    __main__.warning_function
85 items passed all tests:
   3 tests in __main__.example_0
   6 tests in __main__.example_1
[...]
   4 tests in __main__.example_43
   4 tests in __main__.example_44
   4 tests in __main__.example_45
   4 tests in __main__.example_46
   4 tesException RuntimeError: RuntimeError('maximum recursion depth exceeded while calling a Python object',) in  ignored
Exception RuntimeError: RuntimeError('maximum recursion depth exceeded while calling a Python object',) in  ignored
Exception RuntimeError: RuntimeError('maximum recursion depth exceeded while calling a Python object',) in  ignored
Exception RuntimeError: RuntimeError('maximum recursion depth exceeded while calling a Python object',) in  ignored
Exception RuntimeError: RuntimeError('maximum recursion depth exceeded while calling a Python object',) in  ignored
Exception RuntimeError: RuntimeError('maximum recursion depth exceeded while calling a Python object',) in  ignored
[...goes on forever until killed...]

[jdemeyer]

This bug is now prominently in my top 3 of "weirdest Sage bugs I countered as release manager", together with #12221 and #10609.

[vbraun]

My desktop does about 10 tickets/hour, each of which adds a $SAGE_ROOT/devel/sage-<ticket> directory of about 330MB. Is there a reason why the patchbot does not delete these? It kind of adds up, we are talking about >50 GB per day. Or should I get another harddisk to run the patchbot on?

About the maxima_abstract.py error, I never observed it here. It might be a 32-bit issue (I'm running Fedora 17 x86_64). Although perhaps far-fetched, there is another bug where the sage-maxima interface goes into an infinite recursion (#13097).

Even with these issues I'd be in favor of merging this ticket. Its useful functionality and it works for most. We can polish it in a follow-up ticket.

[robertwb]

The patchbot deletes tickets directories when they are closed. It might be worth having an option to delete them after every run, but then a full re-clone and re-build would be required to test any additional patches (expensive).

As for space, all the build and .c files are hard linked from sage-main (unless they're changed), so the aggregate use of the directories is about 100MB each ticket (not each run). Even if we got up to, say, 500 open tickets you are looking at a stead state of 50GB total, not per day.

It will only apply patches to the sage-main tree, which it re-clones for every ticket, but the doctests and code itself could do anything, so one can't rule out any global state (though if there it's both strange and a bug).

I'm all in favor of getting this in and iterating on it there. I made some minor changes and will try to refresh these patches shortly.

[vbraun]

My patchbot processed 236 tickets so far and I'm at 80 gigabytes:

[patchbot@volker-desktop sage-5.1.beta3]$ du -sh .
79G	.
[patchbot@volker-desktop sage-5.1.beta3]$ mount | grep /home
/dev/sdc1 on /home type ext4 (rw,relatime,seclabel,data=ordered)

Thats already accounting for hardlinks, du is smart enough to only count the linked file once. Turning this off (and overcounting) yields

[patchbot@volker-desktop sage-5.1.beta3]$ du -shl .
336G	.

So it should probably stop just below 100G, still acceptable I think.

[robertwb]

Hmm... that is quite a bit more than I'd expect. Certainly keeping only the most recently used N tickets would be an improvement, but between moving to git and cycache and ccache it will become irrelevant (one would simply specify the desired cache size) and 100G isn't overkill these days.

[davidloeffler]

My experience with running patchbot has been that many tickets (notably those that add new modules to the reference manual) do seem to pollute the global state, particularly for documentation building purposes, where the docbuild log for running patchbot on ticket X will start complaining that it can't find a file that is added in some other ticket Y that it previously tested. (I think this is because the sage-clone script hard-links the Sphinx environment pickle.)

Moreover, even without "cross-branch contamination", adding source files and then removing them tends to leave junk in the Sage build tree. Thus the patchbot isn't really doing its job properly, of simulating the effect of applying the latest set of patches to a clean copy of Sage. So I found it preferable to fiddle with the patchbot test_a_ticket code such that it always made a new branch, then deleted it afterwards.

[robertwb]

Well, this is a defect in sage -clone (which should hopefully go away with git). I'm not quite sure how

sage -clone ticket
[test ticket]
sage -b main
sage -clone newticket

would have different behavior from

sage -clone ticket
[test ticket]
sage -b main
rm -rf /path/to/deve/sage-ticket
sage -clone newticket

though I have seen issues with the build directory getting messed up when files are added and then removed.

[vbraun]

With the updated patch I get:

[patchbot@volker-desktop sage-5.1.beta3]$ ./sage -patchbot
/home/patchbot/Sage/sage-5.1.beta3/spkg/bin/sage: line 877: /home/patchbot/Sage/sage-5.1.beta3/local/bin/patchbot/patchbot.py: Permission denied

Manually changing the permission works.

[kini]

apply to $SAGE_LOCAL/bin

[kini]

Replying to vbraun:

With the updated patch I get:

[patchbot@volker-desktop sage-5.1.beta3]$ ./sage -patchbot
/home/patchbot/Sage/sage-5.1.beta3/spkg/bin/sage: line 877: /home/patchbot/Sage/sage-5.1.beta3/local/bin/patchbot/patchbot.py: Permission denied

Manually changing the permission works.

I updated the patch.

[kini]

Hah - patchbot gives its own code a green circle despite there being no patches for the Sage library :)

[vbraun]

Looks good to me!

The perfect is the enemy of the good.

[kini]

On arando I am consistently getting a doctest failure in devel/sage-0/sage/rings/finite_rings/integer_mod.pyx when the patchbot first starts up and tests a vanilla Sage. When I doctest that particular file manually, there are no failures. Even when I run make ptestlong, there is no failure. But if I run sage --patchbot, it fails on that file. This has been happening since 5.1.beta5, as far as I can tell.

[kini]

I tried it again while redirecting stdout to a text file which I later searched. The actual failure is apparently this:

sage -t  -force_lib devel/sage-0/sage/rings/finite_rings/integer_mod.pyx
**********************************************************************
File "/opt/patchbot-5.1.beta6/devel/sage-0/sage/rings/finite_rings/integer_mod.pyx", line 2855:
    sage: mod(5,13^5) == mod(5,13)                                                       
Expected:
    True
Got:
    False
**********************************************************************
1 items had failures:
   1 of   8 in __main__.example_88
***Test Failed*** 1 failures.

[vbraun]

Keshav: Try make ptest or sage -tp <n> $SAGE_ROOT/devel/sage to test multiple files and see if you can reproduce it. The patchbot doesn't do anything different.

[kini]

I've tried this three or four times with the patchbot and three or four times with make ptestlong, and of these trials all the patchbot runs end with this failure, and all the make ptestlongs pass all doctests. I'm trying again now with a plain make ptest (without the long doctests).

[kini]

Yup, no errors with make ptest. Now trying sage --patchbot again for about the fifth time :)

[jdemeyer]

May I remind you about my feature request for "patchbot log grepping": given a doctest error, I would like to find out whether the patchbot has seen that failure and which ticket caused it.

[kini]

sage --patchbot fails again on the same file. I think it's pretty clear that this is only happening when the patchbot runs the tests, though I can't imagine why. Is it worth setting the ticket to needs_work? Can anyone else reproduce this? (Try a 32-bit machine, I suppose...)

[vbraun]

The patchbot just runs sage -t, its probably something in the doctesting framework that needs fixing. There is also the new doctesting framework at ​http://trac.sagemath.org/12415, maybe that'll fix it? In any case that discussion belongs to a different ticket.

Same for feature requests, Jeroen ;-)

[jdemeyer]

It seems that sage-make_devel_packages (I guess) needs to be updated, the patchbot files in local/bin are not picked up by sage-sdist.

[robertwb]

Replying to jdemeyer:

May I remind you about my feature request for "patchbot log grepping": given a doctest error, I would like to find out whether the patchbot has seen that failure and which ticket caused it.

Yes, that would be nice. At the moment my energies are focused on (finally) trying to just get this in.

[robertwb]

Replying to kini:

sage --patchbot fails again on the same file. I think it's pretty clear that this is only happening when the patchbot runs the tests, though I can't imagine why. Is it worth setting the ticket to needs_work? Can anyone else reproduce this? (Try a 32-bit machine, I suppose...)

I have no idea, but unless this is reproducible by others I don't think this should block it getting in.

[kini]

I agree. Volker actually told me on IRC that he couldn't reproduce it on a 32-bit VM, so it must be something peculiar to arando.

[robertwb]

Indeed, it's very strange, as it's just running the same sage -t in a subshell.

[robertwb]

Apply to $SAGE_LOCAL/bin.

[robertwb]

Wow, such ugliness here. I can't wait until we're set up with a unified repository...

[vbraun]

For the record, this is the 32-bit test VM passing the test: ​http://patchbot.sagemath.org/ticket/?machine=Fedora/16/i686/3.4.2-1.fc16.i686.PAE/sagevm&status=open In any case, this should not block the patchbot ticket going forward.

I noticed that $DOT_SAGE/cache contains a file $SAGE_ROOT-$PID-lazy_import_cache.pickle that seems to be not cleaned up by the sage cleaner. So over time the patchbot will fill it with 32k files, one per possible pid. Not terrible, but could be nicer. Should probably be fixed in the sage-cleaner, though.

Running the patchbot for a long time seems to leak file descriptors for a pseudoterminal, after a few days I have

[root@volker-desktop 2040]# ps -p 2040 u
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
patchbot  2040  0.0  0.3 301676 99940 pts/2    S+   Jun30   0:57 python /mnt/storage2TB/patchbot/Sage/sage-5.1.rc0/local/bin/patchbot/patchbot.py
[root@volker-desktop 2040]# ls -alv /proc/2040/fd/*
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/0 -> /dev/pts/2
l-wx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/1 -> pipe:[3598117]
l-wx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/2 -> pipe:[3598117]
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/3 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/4 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/5 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/6 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/7 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/8 -> /dev/pts/2
[...]
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/319 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/320 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 13:14 /proc/2040/fd/321 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/322 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/323 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 12:54 /proc/2040/fd/324 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 13:16 /proc/2040/fd/325 -> /dev/pts/2
lrwx------. 1 patchbot patchbot 64 Jul  1 13:16 /proc/2040/fd/326 -> /dev/pts/2
l-wx------. 1 patchbot patchbot 64 Jul  1 13:16 /proc/2040/fd/328 -> pipe:[3598117]

[vbraun]

PS: there are no spurious processes around as far as I can tell

[root@volker-desktop fd]# pstree -p 2040
python(2040)─┬─bash(19705)───bash(19707)───python(19710)─┬─python(19711)───bash(22440)───python(22441)───python(22442)
             │                                           ├─python(19712)───bash(22317)───python(22318)───python(22319)─┬─gp(22346)
             │                                           │                                                             ├─gp(22364)
             │                                           │                                                             ├─mwrank(22334)
             │                                           │                                                             ├─mwrank(22337)
             │                                           │                                                             ├─mwrank(22340)
             │                                           │                                                             └─mwrank(22443)
             │                                           ├─python(19713)───bash(22450)───python(22451)───python(22452)
             │                                           ├─{python}(19714)
             │                                           ├─{python}(19715)
             │                                           └─{python}(19716)
             ├─python(19692)
             └─tee(19283)
[root@volker-desktop fd]# ps -p 19705 u
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
patchbot 19705  0.0  0.0 112096  1316 pts/2    S+   13:17   0:00 bash /mnt/storage2TB/patchbot/Sage/sage-5.1.rc0/sage -tp 3 -sagenb /mnt/storage2TB/patchbot/S
[root@volker-desktop fd]# ps -p 19692 u
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
patchbot 19692  0.0  0.0      0     0 pts/2    Z+   13:17   0:00 [python] <defunct>
[root@volker-desktop fd]# ps -p 19283 u
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
patchbot 19283  0.0  0.0 106848   624 pts/2    S+   13:16   0:00 tee /mnt/storage2TB/patchbot/Sage/sage-5.1.rc0/logs/10764-log.txt

[vbraun]

Also, for the life of me I can't figure out how to make the patchbot ignore the unused patches in #12544. Am I missing anything? Perhaps we need to support "ignore file.patch" in the comment text to explicitly tell the bot to ignore patches?

[vbraun]

Initial patch

[vbraun]

The attached patch fixes the fd leak (2 per ticket) for me.

[vbraun]

I'm fine with the ticket going into Sage. Positive review for everything except my patch.

[kini]

For what it's worth, I'm no longer getting the failure on arando after I rebooted it and installed this ticket into 5.1.rc0 (I just got into Singapore today so I was able to physically access the machine again), so you can forget about that, I guess...

[robertwb]

Ah, thanks for the file descriptor leak! Positive review on that patch. I'll look into the issue at #12544, but that seems to be an exceptional case.

[jdemeyer]

sage-sdist isn't fixed at all, the following files still disappear:

! patchbot/README.txt
! patchbot/db.py
! patchbot/http_post_file.py
! patchbot/images/blue-blob.png
! patchbot/images/green-blob.png
! patchbot/images/purple-blob.png
! patchbot/images/red-blob.png
! patchbot/images/white-blob.png
! patchbot/images/yellow-blob.png
! patchbot/patchbot.py
! patchbot/plugins.py
! patchbot/run_server.py
! patchbot/serve.py
! patchbot/templates/base.html
! patchbot/templates/ticket.html
! patchbot/templates/ticket_list.html
! patchbot/trac.py
! patchbot/util.py

[robertwb]

Argh! OK, I'm going to rebase this on an entirely clean copy of Sage and git it fixed tonight.

[robertwb]

I'm not sure I see what the problem is; is the issue that the files aren't extracted in the spkg itself? That shouldn't matter at all, as they're in the history which is what's used to install the spkg.

[jdemeyer]

The issue is that the files are not copied into the spkg by sage-sdist.

[robertwb]

My question is more why do they need to be?

[jdemeyer]

Replying to robertwb:

My question is more why do they need to be?

Because we have always done things that way, and I don't see any reason why we should make an exception for the patchbot files.

Surely our installation/packaging procedure can be improved, but that's the topic of #13015, not of this ticket.

[jdemeyer]

If you really want to change the installation of the scripts spkg, make a separate ticket and add that as dependency here.

[robertwb]

Sure, I'll do that. (I just did that here as it seemed cleaner than explicitly adding a bunch of globs to copy, making sure I got everything we wanted and nothing we didn't.)

[jdemeyer]

* ping *

[jdemeyer]

Does the patchbot already feature the automatic deleting of old devel/sage-NNNNN directories?

[kini]

It doesn't do so by default at the moment, at least.

[robertwb]

Sorry I haven't gotten back to this. I'll try to get to it within the week. Thanks for the ping.

Regarding deleting the sage-NNNNN directories, It should delete them as the tickets get closed.

[jdemeyer]

Just an idea concerning the distribution of the patchbot: maybe you could make a separate spkg for the patchbot instead of distributing it with sage_scripts?

[jdemeyer]

Replying to robertwb:

Regarding deleting the sage-NNNNN directories, It should delete them as the tickets get closed.

I would propose something like: if there is no activitity on a ticket (meaning no patches are changed) for a certain time (one week?), then also delete the directory.

[vbraun]

I've been running a patchbot for a while now and disk usage isn't really that much of a deal. Especially since there isn't any reason to keep old versions around as development goes on. What is much more annoying is that this ticket is still not merged so with each beta I have to install a handful of patches on a machine on the other side of the atlantic...

[kini]

I have this setup on arando:

[1] patchbot@arando /opt $ ls -hal
total 4.4G
drwxrwxr-x  3 root     opt      4.0K Aug 15 21:42 .
drwxr-xr-x 22 root     root     4.0K Aug 13 18:32 ..
lrwxrwxrwx  1 patchbot patchbot   18 Aug 13 20:12 patchbot -> patchbot-5.3.beta1
drwxr-xr-x 10 patchbot patchbot 4.0K Aug 13 23:49 patchbot-5.3.beta1
-rw-rw-r--  1 patchbot patchbot 295M Jul  3 03:15 sage-5.0.1.tar
-r--r--r--  1 kini     kini     294M May 15 17:54 sage-5.0.tar
-rw-r--r--  1 kini     kini     294M May 18 02:09 sage-5.1.beta0.tar
-rw-rw-r--  1 patchbot patchbot 294M May 29 05:40 sage-5.1.beta1.tar
-rw-rw-r--  1 patchbot patchbot 290M Jun  4 00:58 sage-5.1.beta2.tar
-rw-rw-r--  1 patchbot patchbot 290M Jun  7 17:37 sage-5.1.beta3.tar
-rw-rw-r--  1 patchbot patchbot 291M Jun 19 19:31 sage-5.1.beta5.tar
-rw-rw-r--  1 patchbot patchbot 291M Jun 25 19:30 sage-5.1.beta6.tar
-rw-rw-r--  1 patchbot patchbot 291M Jul  3 03:16 sage-5.1.rc0.tar
-rw-rw-r--  1 patchbot patchbot 291M Jul  6 07:32 sage-5.1.rc1.tar
-rw-rw-r--  1 patchbot patchbot 313M Jul 10 02:18 sage-5.2.beta0.tar
-rw-rw-r--  1 patchbot patchbot 314M Jul 25 01:59 sage-5.2.rc1.tar
-rw-rw-r--  1 patchbot patchbot 314M Jul 26 02:00 sage-5.2.tar
-rw-rw-r--  1 patchbot patchbot 314M Aug  1 23:55 sage-5.3.beta0.tar
-rw-rw-r--  1 patchbot patchbot 314M Aug 13 06:15 sage-5.3.beta1.tar
-rwxrwxr-x  1 patchbot patchbot 1.3K Aug 15 21:42 setup-patchbot.sh
[1] patchbot@arando /opt $ cat setup-patchbot.sh 
#!/bin/bash

# This script upgrades the currently running patchbot in this directory.
# Supply a Sage version number as an argument, and the script will
# download, set up, and run the patchbot.

VER=$1
[ -n $VER ] || exit 1

# Get new patchbot and extract it
rm -f sage-$VER.tar
wget http://boxen.math.washington.edu/home/release/sage-$VER/sage-$VER.tar
rm -rf $(readlink patchbot)

# Delete old patchbot dir (keep tarball) and move symlink to new
# patchbot dir
rm patchbot
tar xf sage-$VER.tar
mv sage-$VER patchbot-$VER
ln -s patchbot-$VER patchbot

# Build new patchbot's Sage from source
cd patchbot
make

# Install the patchbot into the newly built Sage version (TODO: update
# this when the patches on ticket #12486 change, or when #12486 is
# merged)
hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12486/patchbot-root-5.0.patch
hg -R local/bin qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12486/12486-patchbot-scripts-5.0-updated.patch
hg -R local/bin qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12486/12486-fix-sdist.patch
hg -R local/bin qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12486/trac_12486_fix_fd_leak.patch
./sage -b

# Start the patchbot
exec ./sage --patchbot
[1] patchbot@arando /opt $ 

Makes it pretty easy...

[robertwb]

I've created an spkg: ​http://sage.math.washington.edu/home/robertwb/patches/patchbot-1.0.spkg

The contents of this spkg should be identical to the (flattened) patches.

[jdemeyer]

Adding a spkg requires changes to spkg/standard/deps and spkg/install.

Leaving $SAGE_LOCAL unquoted is not so good:

$SAGE_LOCAL/bin/patchbot/patchbot.py

Better is

"$SAGE_LOCAL/bin/patchbot/patchbot.py"

[kini]

As Simon King pointed out on sage-devel a few days ago, the patchbot shouldn't report "ApplyFailed" when what failed was the downloading of the patches rather than the application of the patches.

[vbraun]

Good idea to package the bot. I think we should start by making the patchbot an optional spkg. We don't need to modify spkg/standard/deps or add id to the sage -advanced help then. Once it is an optional spkg we can vote on sage-devel to make it standard (+1 from me).

For consistency the mercurial repo inside the spkg should have the bot committed and not as a patch queue. Apart from that its ready to go.

[kini]

The Mercurial repo inside the SPKG should not have the bot committed. The bot should be in the src/ directory and the src/ directory should be ignored in .hgignore. This is standard among SPKGs, and is also what is present in the current patch queue in the SPKG.

[vbraun]

Thats what I meant. Sorry for not being clear. My point is that the SPGK.txt, ... files are in a mercurial queue right now.

[vbraun]

patchbot.py is not executable, so sage --patchbot fails to execute.

[robertwb]

OK, I've pushed a new commit to make patchbot.py executable and make the hg commit a real commit. It's nit-picky stuff like this that in part made me want to avoid making an spkg. I'd much rather spend time improving or cleaning up the patchbot than worrying about packaging details as I've done over the last 7 (!) months since filing this ticket (whether as a patch or an spkg) but it's a real pain to not have this in. Basically, we need to move to a better system soon... OK, done ranting.

I also made it to qimport failing doesn't cause the ticket to fail.

[robertwb]

And, to be clear, no offense to any of you intended, in fact I'm really grateful you're looking at this. It's the system that's to onerous.

[vbraun]

Looks good to me!

[schilly]

the optional spkg is on the server+mirrors now.