Opened 9 years ago

Closed 9 years ago

#13457 closed enhancement (fixed)

Various small fixes to sage-bdist

Reported by: jdemeyer Owned by: leif
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 jdemeyer)

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

Attachments (1)

13457_sage_bdist.patch (4.6 KB) - added by jdemeyer 9 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 9 years ago by jdemeyer

  • Summary changed from Some refactoring of sage-bdist to Various small fixes to sage-bdist

comment:2 Changed 9 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from new to needs_review

comment:3 Changed 9 years ago by jhpalmieri

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 9 years ago by jdemeyer

Fixed.

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

comment:5 Changed 9 years ago by jhpalmieri

The new patch looks identical to the old patch.

comment:6 Changed 9 years ago by jdemeyer

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

comment:7 Changed 9 years ago by jdemeyer

Fixed (for real this time).

comment:8 Changed 9 years ago by jhpalmieri

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 9 years ago by jdemeyer

comment:9 Changed 9 years ago by jdemeyer

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 9 years ago by jhpalmieri

  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

Okay, good.

comment:11 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.4.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.