William Stein wrote: > I think a huge amount of the problem will be that many of those 66
<SNIP> > positive reviewed tickets probably will: > (2) many of the patches will probably fail on 32-bit or ppc or OS X, > even though they worked fine on sage.math. In a post yesterday I said: ------------------------------------------------------------------ For Unix/Linux environments, perhaps a sensible solution could be summed up in two or perhaps three sentences. 1) Ensure you respect all environment variables. 2) Check it works on all supported operating systems using GCC as a compiler. 3) Check it works on Cygwin, since a port to this is planned. ----------------------------------------------------------------- You said "Definitely your 1-3 are definitely a good idea though... it's hard to argue with them." Would it not be sensible to ask people submitting patches/spkgs to test them on those platforms, and show they work, by attaching logs? You can't expect people to own all that hardware, but if you can allow people access to what you have, then this is perhaps one way to solve the problem why "many of the patches will fail". It really does help improve quality too. Despite compiling libm4ri on numerous occasions on multi-processor machine, I'd never noticed the code for determining the number of CPUs would not work on Solaris. It was only when I tried building libm4ri on HP-UX that I realised there was a problem, as there the configure script actually broke. This got me to post to the autoconf mailing list, providing a link to the macro on the GNU site Here are a couple of the comments about that macro used in libm4ri, which was taken from the autoconf macro archive http://www.nongnu.org/autoconf-archive/ax_count_cpus.html 1) Ralf Wildenhues said: "This macro does not look well-designed. It does the wrong thing when cross-compiling, or even only compiling for a different system (which happens to be use the same architecture)" 2) Mike Frysinger said "there are so many reasons why this is wrong" So in summary * The macro will not work on Solaris, which is a supported operating systems, though I had never noticed it. * The failure to even run on HP-UX got me to look at the code in detail. * Both of the above issues got me to look at other parts of the configure.ac used by libm4ri, and realise the code for determining the cache sizes was incorrect too. So testing on an unsupported platform identified problems on at least one supported platform (Solaris) and I think it will be broken on PPC too. Testing with Sun Studio highlighted yet another issue with that configure script in lib4mri - it thinks the Sun compiler can't create executables. With 90% of developers using linux, perhaps sage.math is the machine were there is least point in actually testing the code. How about this idea 1) All developers state what is their main platform they build on (in my case Solaris). 2) When submitting a patch, they attach logs showing building on all platforms, *except* the one they normally work on, since one can assume they have probably got their patch working there. In which case, I would attach logs showing it working on Linux, OS X and if possible Cygwin [note 1]. I would *not* attach a log for Solaris. This really is no more than an extension of the review process, where the submitter of a patch has to convince the reviewer the code works on the other platforms. That should increase the number of positive reviews where the code actually makes it into Sage. Dave Notes [1] I'm not sure how far advanced the port on Cygwin is, so how practical that is. Dave --~--~---------~--~----~------------~-------~--~----~ To post to this group, send an email to sage-devel@googlegroups.com To unsubscribe from this group, send an email to sage-devel-unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/sage-devel URL: http://www.sagemath.org -~----------~----~----~----~------~----~------~--~---
