#11232: we should not build patch on Cygwin
----------------------------+-----------------------------------------------
Reporter: dimpase | Owner: tbd
Type: defect | Status: needs_review
Priority: major | Milestone: sage-4.7
Component: cygwin | Keywords:
Work_issues: | Upstream: N/A
Reviewer: David Kirkby | Author:
Merged: | Dependencies:
----------------------------+-----------------------------------------------
Comment(by drkirkby):
Replying to [comment:9 dimpase]:
> Replying to [comment:6 drkirkby]:
> > If Cygwin needs ''patch'' installed this should be checked early on.
We test that a Fortran compiler exists well before we start to use it. Why
should ''patch'' be any different?
>
> why should patch be different from Atlas?
Because the ATLAS method is incorrect. That's a good enough reason for me.
Two wrongs don't make a right.
The ATLAS spkg-install happens to be very poor in many respects. It is a
very small Python script that calls a large bash script. I don't think we
should use ATLAS as a model.
> I went the way it was done for Atlas.
Numerous tests for programs are performed - gcc, g++, gfortran, m4, bash.
There's a test for the maths library too. All these checks for
prerequisites are done at the start. Just because one package happens to
do it the wrong way, does not mean we should copy that.
> OK, there is an inconsistency in design here, as you point out,
> but it's a minor and fixable one---if one has plenty of time and a
Windows machine at hand to test things. But the gain would be small.
If it's minor and fixable is should be fixed. I don't think its a good
idea to let a build progress any further than necessary if it is obvious
it is going to fail.
> > Exiting on Cygwin is sensible if there' no need to install ''patch'',
but testing if the program exists should in my opinion be done much
earlier on. I believe the way to do that is probably to use
{{{AC_CHECK_PROG}}} in the 'prereq' part of Sage.
> >
> > Also note running ''patch'' with no arguments will leave it there
sitting for input.
>
> well, yes, it would. Hence there is an argument...
> > Can you attach a Mercurial patch for review purposes, so we can see
what you are trying to do. The ticket is much more informative if it has
the changes attached.
>
> if you untar the spkg file and do
> {{{
> hg diff -r3:6
> }}}
>
> you will see the difference. Do we need to post perfectly reproducible
things like this here?
Well 99% of people do attach patches, which makes review a lot easier. It
also leaves a record for others to see what is proposed and why specific
solutions were rejected.
Anyway, I'm not happy with this approach to solving an issue with
''patch'', when it's clear there are better ways that don't copy a method
that's clearly flawed. I'm not going to put it back to "needs_work" again,
as clearly you feel differently. You might find another reviewer who is
happy with this method.
Dave
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11232#comment:10>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
