Opened 10 years ago

Closed 10 years ago

#13457 closed enhancement (fixed)

Various small fixes to sage-bdist

Reported by: Jeroen Demeyer Owned by: Leif Leonhardy
Priority: major Milestone: sage-5.4
Component: scripts Keywords:
Cc: Merged in: sage-5.4.beta2
Authors: Jeroen Demeyer Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

See patch, apply 13457_sage_bdist.patch to the scripts repository.

Attachments (1)

13457_sage_bdist.patch (4.6 KB) - added by Jeroen Demeyer 10 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 10 years ago by Jeroen Demeyer

Summary: Some refactoring of sage-bdistVarious small fixes to sage-bdist

comment:2 Changed 10 years ago by Jeroen Demeyer

Description: modified (diff)
Status: newneeds_review

comment:3 Changed 10 years ago by John Palmieri

If you feel like doing a little more:

  • sage-bdist

    diff --git a/sage-bdist b/sage-bdist
    a b TARGET=sage-"$SAGE_VERSION"-`uname -m`-` 
    3232TARGET=`echo $TARGET | sed 's/ //g'`   # Remove spaces
    3333TMP="$CUR/tmp/$TARGET"
    3434
    35 mkdir -p "$CUR/tmp"
    36 
    3735rm -rf "$TMP"
    38 mkdir "$TMP"
     36mkdir -p "$TMP"
    3937
    4038# copy sage root repo over:
    4139cd "$SAGE_ROOT"
    if [ -d devel/sage ]; then 
    7270    ln -sf ../../../../devel/sage/build/sage .
    7371fi
    7472
     73cd "$SAGE_ROOT"
     74
    7575if [ -d devel/sagenb ]; then
    7676    echo "Copying Sage Notebook"
    7777    cp $CP_OPT -L devel/sagenb "$TMP/devel/sagenb-main"
    7878    ln -s sagenb-main "$TMP/devel/sagenb"
    7979fi
    8080
    81 
    82 cd "$SAGE_ROOT"
    83 
    8481if [ -d "$PKGDIR" ]; then
    8582   echo "Making empty spkg's"
    8683   cd "$PKGDIR"

The last two changes are actually crucial: you need a cd "$SAGE_ROOT" before line 75 because otherwise, the working directory is wrong and the if [ -d devel/sagenb ] block doesn't execute.

I'll try to keep looking at this.

comment:4 Changed 10 years ago by Jeroen Demeyer

Fixed.

Note that the main motivation for this ticket is as prerequisite of #13123.

comment:5 Changed 10 years ago by John Palmieri

The new patch looks identical to the old patch.

comment:6 Changed 10 years ago by Jeroen Demeyer

I accidentally made the fixes to the wrong patch. I'm working on too many tickets at the same time...

comment:7 Changed 10 years ago by Jeroen Demeyer

Fixed (for real this time).

comment:8 Changed 10 years ago by John Palmieri

When I run this on OpenSolaris, I see lots of warning messages like

Copying files over to tmp directory
cp: Failed to set acl entries on /export/home/palmieri/testing/sage-5.3.beta2/tmp/sage-5.3.X-i86pc-SunOS//local/include/csage
cp: Failed to set acl entries on /export/home/palmieri/testing/sage-5.3.beta2/tmp/sage-5.3.X-i86pc-SunOS//local/lib/libreadline.so

As far as I can tell, the only consequence of this is that the various symbolic links in local/lib end up with new modification times. If I'm right, we can ignore the warnings. The resulting binary seems to work just fine. What do you think?

Also, the verbose flag in tar zcvf ... is unneeded, in my opinion, although I guess it's nice to see some indication of progress while the tar.gz file is being created.

Changed 10 years ago by Jeroen Demeyer

Attachment: 13457_sage_bdist.patch added

comment:9 Changed 10 years ago by Jeroen Demeyer

I haven't looked at the OpenSolaris issues, but in any case it's not a regression of this ticket.

Agree on removing the tar v flag, new patch needs review.

comment:10 Changed 10 years ago by John Palmieri

Reviewers: John Palmieri
Status: needs_reviewpositive_review

Okay, good.

comment:11 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.4.beta2
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.