Opened 9 years ago
Closed 9 years ago
#12486 closed enhancement (fixed)
Make the Sage patchbot an optional spkg
Reported by: | robertwb | Owned by: | mvngu |
---|---|---|---|
Priority: | major | Milestone: | sage-5.3 |
Component: | packages: optional | Keywords: | |
Cc: | kini, dkrenn | Merged in: | sage-5.3.rc1 |
Authors: | Robert Bradshaw, Dan Drake | Reviewers: | Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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
Attachments (7)
Change History (92)
comment:1 Changed 9 years ago by
- Status changed from new to needs_review
Changed 9 years ago by
comment:2 Changed 9 years ago by
comment:3 Changed 9 years ago by
- Cc kini added
comment:4 Changed 9 years ago by
- Description modified (diff)
I split the original patch, since in 5.0, the sage
script is tracked by the root repo.
comment:5 Changed 9 years ago by
We should document the patchbot. Something better than "read the code" and "ask Robert on sage-devel". This is now #12505.
comment:6 Changed 9 years ago by
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. :)
comment:7 Changed 9 years ago by
- Cc dkrenn added
comment:8 Changed 9 years ago by
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.
comment:9 follow-up: ↓ 10 Changed 9 years ago by
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.
comment:10 in reply to: ↑ 9 Changed 9 years ago by
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.)
comment:11 Changed 9 years ago by
Thanks. I noticed that too.
comment:12 Changed 9 years ago by
- Description modified (diff)
comment:13 Changed 9 years ago by
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
comment:14 Changed 9 years ago by
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.
comment:15 Changed 9 years ago by
kini: I'd love to know which ticket causes the maxima_abstract failures.
comment:16 Changed 9 years ago by
By the way: the "Python keeps running after a doctest timeout" bug is #11338.
comment:17 Changed 9 years ago by
[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-*
.)
comment:18 Changed 9 years ago by
There is no logic behind this, too many independent tickets have this issue.
comment:19 Changed 9 years ago by
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.
comment:20 Changed 9 years ago by
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...]
comment:21 Changed 9 years ago by
comment:22 Changed 9 years ago by
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.
comment:23 Changed 9 years ago by
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.
comment:24 Changed 9 years ago by
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.
comment:25 Changed 9 years ago by
- Description modified (diff)
comment:26 Changed 9 years ago by
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.
comment:27 Changed 9 years ago by
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.
comment:28 Changed 9 years ago by
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.
comment:29 follow-up: ↓ 30 Changed 9 years ago by
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.
comment:30 in reply to: ↑ 29 Changed 9 years ago by
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 deniedManually changing the permission works.
I updated the patch.
comment:31 Changed 9 years ago by
Hah - patchbot gives its own code a green circle despite there being no patches for the Sage library :)
comment:32 Changed 9 years ago by
- Reviewers set to Volker Braun
- Status changed from needs_review to positive_review
Looks good to me!
The perfect is the enemy of the good.
comment:33 Changed 9 years ago by
- Milestone changed from sage-5.1 to sage-5.2
comment:34 Changed 9 years ago by
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.
comment:35 Changed 9 years ago by
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.
comment:36 Changed 9 years ago by
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.
comment:37 Changed 9 years ago by
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 ptestlong
s pass all doctests. I'm trying again now with a plain make ptest
(without the long doctests).
comment:38 Changed 9 years ago by
Yup, no errors with make ptest
. Now trying sage --patchbot
again for about the fifth time :)
comment:39 follow-up: ↓ 43 Changed 9 years ago by
- Summary changed from Add patchbot to Sage itself. to Add patchbot to Sage itself
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.
comment:40 follow-up: ↓ 44 Changed 9 years ago by
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...)
comment:41 Changed 9 years ago by
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 ;-)
comment:42 Changed 9 years ago by
- Status changed from positive_review to needs_work
- Work issues set to sage-sdist
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
.
comment:43 in reply to: ↑ 39 Changed 9 years ago by
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.
comment:44 in reply to: ↑ 40 Changed 9 years ago by
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.
comment:45 Changed 9 years ago by
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.
comment:46 Changed 9 years ago by
Indeed, it's very strange, as it's just running the same sage -t in a subshell.
comment:47 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
Wow, such ugliness here. I can't wait until we're set up with a unified repository...
comment:48 Changed 9 years ago by
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]
comment:49 Changed 9 years ago by
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
comment:50 Changed 9 years ago by
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?
comment:51 Changed 9 years ago by
- Description modified (diff)
The attached patch fixes the fd leak (2 per ticket) for me.
comment:52 Changed 9 years ago by
- Work issues sage-sdist deleted
I'm fine with the ticket going into Sage. Positive review for everything except my patch.
comment:53 Changed 9 years ago by
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...
comment:54 Changed 9 years ago by
- Status changed from needs_review to positive_review
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.
comment:55 Changed 9 years ago by
- Status changed from positive_review to needs_work
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
comment:56 Changed 9 years ago by
Argh! OK, I'm going to rebase this on an entirely clean copy of Sage and git it fixed tonight.
comment:57 Changed 9 years ago by
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.
comment:58 Changed 9 years ago by
The issue is that the files are not copied into the spkg by sage-sdist
.
comment:59 follow-up: ↓ 60 Changed 9 years ago by
My question is more why do they need to be?
comment:60 in reply to: ↑ 59 Changed 9 years ago by
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.
comment:61 Changed 9 years ago by
If you really want to change the installation of the scripts spkg, make a separate ticket and add that as dependency here.
comment:62 Changed 9 years ago by
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.)
comment:63 Changed 9 years ago by
* ping *
comment:64 Changed 9 years ago by
Does the patchbot already feature the automatic deleting of old devel/sage-NNNNN directories?
comment:65 Changed 9 years ago by
It doesn't do so by default at the moment, at least.
comment:66 follow-ups: ↓ 67 ↓ 68 Changed 9 years ago by
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.
comment:67 in reply to: ↑ 66 Changed 9 years ago by
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
?
comment:68 in reply to: ↑ 66 Changed 9 years ago by
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.
comment:69 Changed 9 years ago by
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...
comment:70 Changed 9 years ago by
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...
comment:71 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
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.
comment:72 Changed 9 years ago by
- Status changed from needs_review to needs_work
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"
Changed 9 years ago by
comment:73 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:74 Changed 9 years ago by
- Description modified (diff)
comment:75 Changed 9 years ago by
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.
comment:76 Changed 9 years ago by
- Summary changed from Add patchbot to Sage itself to Make the Sage patchbot an optional spkg
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.
comment:77 Changed 9 years ago by
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.
comment:78 Changed 9 years ago by
Thats what I meant. Sorry for not being clear. My point is that the SPGK.txt
, ... files are in a mercurial queue right now.
comment:79 Changed 9 years ago by
patchbot.py
is not executable, so sage --patchbot
fails to execute.
comment:80 Changed 9 years ago by
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.
comment:81 Changed 9 years ago by
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.
comment:82 Changed 9 years ago by
- Component changed from doctest to optional packages
comment:83 Changed 9 years ago by
- Status changed from needs_review to positive_review
Looks good to me!
comment:84 Changed 9 years ago by
the optional spkg is on the server+mirrors now.
comment:85 Changed 9 years ago by
- Merged in set to sage-5.3.rc1
- Resolution set to fixed
- Status changed from positive_review to closed
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 toSAGE_ROOT/spkg/bin/sage
for 5.0.