Opened 12 years ago

Closed 12 years ago

#4261 closed defect (fixed)

[with patch, positive review] sympow Configure fails to handle aliases

Reported by: wjp Owned by: mabshoff
Priority: major Milestone: sage-3.2.1
Component: build Keywords:
Cc: dkohel Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

The sympow Configure script has a whichexe function to determine which rm, grep, etc to call that effectively does RM=`which rm`. If rm is an alias (e.g., aliased to rm -i), this fails.

Attachments (1)

trac4261_sympow_Configure.patch (626 bytes) - added by wjp 12 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 12 years ago by wjp

Forgot to mention: this was reported by David Kohel in Nancy during SD10.

Changed 12 years ago by wjp

comment:2 Changed 12 years ago by wjp

  • Summary changed from sympow Configure fails to handle aliases to [with patch, needs review] sympow Configure fails to handle aliases

comment:3 Changed 12 years ago by mabshoff

  • Milestone changed from sage-3.2 to sage-3.1.3

This will be fixed in 3.1.3 :)

Cheers,

Michael

comment:4 Changed 12 years ago by mabshoff

  • Milestone changed from sage-3.2.1 to sage-3.2

Sorry that this didn't make it into 3.1.3/4. But I will attempt to get this into 3.2.

Cheers,

Michael

comment:5 Changed 12 years ago by was

  • Summary changed from [with patch, needs review] sympow Configure fails to handle aliases to [with patch, needs work] sympow Configure fails to handle aliases

REFEREE REPORT:

  1. It does not fail is rm is an alias. It gives the original executable with an exact path.
    teragon-2:sympow-1.018.1.p5 wstein$ alias rm="rm -i"
    teragon-2:sympow-1.018.1.p5 wstein$ which rm
    /bin/rm
    teragon-2:sympow-1.018.1.p5 wstein$ RM=`which rm`
    teragon-2:sympow-1.018.1.p5 wstein$ echo $RM
    /bin/rm
    

So I totally don't get what the problem is. The above patch would have the effect of making it so the scripts would annoyingly suddenly actually *be* interactive if one has aliased rm to "rm -i".

So from my point of view, it looks like this patch introduces a bug instead of fixing one.

  1. This patch would replace all the absolute paths to programs to their names, thus completely removing whatever "upstream's" point was in having those variables. That's suspicious.

So I'm dubious.

comment:6 Changed 12 years ago by mabshoff

  • Cc dkohel added

The behavior of "alias" might be shell dependent which might contribute to the problem here. This was initially reported by David Kohel, so let's CC him.

Cheers,

Michael

comment:7 Changed 12 years ago by was

  • Summary changed from [with patch, needs work] sympow Configure fails to handle aliases to [with patch, positive review] sympow Configure fails to handle aliases
Hi,

Based on Mark's remark below, I give #4261 a positive review, since it does
in fact do just what Mark suggests below.

William

On Thu, Nov 27, 2008 at 9:57 PM, Mark Watkins <watkins@maths.usyd.edu.au> wrote:
> William Stein wrote:
>> Do you guy's have any comments on this:
>>    http://trac.sagemath.org/sage_trac/ticket/4261
>> I'm tempted to mark it invalid, but maybe I'm missing the point.
>
> I think I agree that the problem is with the shell-version of alias.
>
> I was only trying to make something that would be more likely to work than
> the simple /bin/rm, etc., but it seems that I failed. Probably it is safe to
> just use $RM=rm (same with $CP, $TAR, $MKDIR, $TOUCH) in the Makefile and
> hope the user has a sufficient path. Also, echo might be shell-dependent.
>
> I think some buildutils simply tree-search the path and/or the
> whole directory structure, but I didn't want to attempt that.
>
> ===
> Mark Watkins
> watkins@maths.usyd.edu.au
>



-- 
William Stein
Associate Professor of Mathematics
University of Washington
http://wstein.org

comment:8 Changed 12 years ago by mabshoff

  • Milestone changed from sage-3.2.2 to sage-3.2.1

This is going into 3.2.1.

Cheers,

Michael

comment:10 Changed 12 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged in Sage 3.2.1.rc1

Note: See TracTickets for help on using tickets.