Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#10156 closed enhancement (fixed)

Rename top-level makefile to Makefile

Reported by: jdemeyer Owned by: tbd
Priority: minor Milestone: sage-4.6.1
Component: distribution Keywords: scripts makefile
Cc: jhpalmieri, leif Merged in: sage-4.6.1.alpha1
Authors: Jeroen Demeyer, John Palmieri Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

This ticket has a few patches which should be applied when renaming makefile to Makefile. See also #9799.

Apply:

  1. 10156_doc.patch
  2. 10156_scripts.patch
  3. install.patch

Attachments (5)

10156_doc.patch (3.4 KB) - added by jdemeyer 9 years ago.
sagelib patch
10156_scripts.patch (2.1 KB) - added by jdemeyer 9 years ago.
sage_scripts patch
10156_upgrade.patch (659 bytes) - added by jdemeyer 9 years ago.
Additional scripts patch: rename makefile to Makefile during upgrade
install (8.9 KB) - added by jhpalmieri 9 years ago.
the file spkg/install
install.patch (521 bytes) - added by jhpalmieri 9 years ago.
patch for 'install'

Download all attachments as: .zip

Change History (32)

Changed 9 years ago by jdemeyer

sagelib patch

Changed 9 years ago by jdemeyer

sage_scripts patch

comment:1 Changed 9 years ago by jdemeyer

  • Authors set to Jeroen Demeyer

comment:2 Changed 9 years ago by leif

  • Cc jhpalmieri leif added

The renaming is also done by #9433 IIRC.

comment:3 follow-up: Changed 9 years ago by leif

  • Status changed from new to needs_review

I wonder if it's safer to use [Mm]akefile in the shell scripts. (The scripts calling Python functions rather than os.system() etc. would get more complicated of course.)

At least there seems to be a lack of error / consistency checking, as always...

comment:4 Changed 9 years ago by leif

Patches look reasonable; I just don't know yet if they catch all instances.

comment:5 Changed 9 years ago by jdemeyer

If you thrust the author of the patch to do the test, I am doing the following:

  • building Sage from source with #9799, #10156, #10157 applied and makefile renamed to Makefile
  • making an sdist and a bdist from this built Sage
  • extract the sdist, make, make ptestlong
  • extract the bdist, make ptestlong

If all this works, I think that #10156 and #10157 are proven to work.

comment:6 Changed 9 years ago by jdemeyer

  • Description modified (diff)

All builds and tests mentioned above worked with #9799, #10156, #10157 applied.

comment:7 in reply to: ↑ 3 Changed 9 years ago by jdemeyer

Replying to leif:

I wonder if it's safer to use [Mm]akefile in the shell scripts.

I don't see why. If we really rename makefile to Makefile, there is no reason to use [Mm]akefile.

comment:8 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.6.1.alpha0

comment:9 Changed 9 years ago by jdemeyer

John or Leif: could you review this and #10157 please? I think these patches have received sufficient testing to prove that they work.

comment:10 follow-up: Changed 9 years ago by jhpalmieri

The Sage library patch looks fine.

For the scripts patch, I haven't tested it at all, but what happens if you upgrade from a version of Sage with a file called "makefile"? Will things break? I don't think it's ever a good idea to run "sage-sdist" or "sage-bdist" from an upgraded version of Sage, but if someone does, what will happen? Would it be a good idea to either use something like [Mm]akefile, as leif suggested, or do something like the following:

if [ -e makefile ]; then
   cp -$OPT makefile "$TMP"/Makefile
fi

(This is for sage-bdist, and you would use something similar in sage-sdist.)

comment:11 in reply to: ↑ 10 Changed 9 years ago by leif

Replying to jhpalmieri:

The Sage library patch looks fine.

For the scripts patch, I haven't tested it at all, but what happens if you upgrade from a version of Sage with a file called "makefile"? Will things break?

W.r.t. making new dists from an upgraded version, hopefully #9433 solves this... ;-)

(I don't know if you meant upgrading in general; this works for me. You just still have the old makefile.)

I don't think it's ever a good idea to run "sage-sdist" or "sage-bdist" from an upgraded version of Sage [...]

Nobody should do this, currently.

We should perhaps just give an error message in that case (until #9433 is merged).

comment:12 follow-up: Changed 9 years ago by jdemeyer

I don't understand the fine details of upgrading, but would it be possible to copy makefile to Makefile during the upgrade?

comment:13 in reply to: ↑ 12 ; follow-up: Changed 9 years ago by leif

Replying to jdemeyer:

I don't understand the fine details of upgrading, but would it be possible to copy makefile to Makefile during the upgrade?

Of course. But I would rename it because (POSIX) make gives makefile precedence over Makefile and we don't want to cause confusion.

comment:14 in reply to: ↑ 13 ; follow-up: Changed 9 years ago by jdemeyer

  • Status changed from needs_review to needs_work
  • Work issues set to copy makefile to Makefile during upgrade

Replying to leif:

Replying to jdemeyer:

I don't understand the fine details of upgrading, but would it be possible to copy makefile to Makefile during the upgrade?

Of course. But I would rename it because (POSIX) make gives makefile precedence over Makefile and we don't want to cause confusion.

On the other hand, if makefile and Makefile are identical files, it shouldn't matter. It seems just dangerous to delete makefile during a Sage build/upgrade.

Can any of you implement this?

comment:15 in reply to: ↑ 14 Changed 9 years ago by jhpalmieri

Replying to jdemeyer:

Replying to leif:

Replying to jdemeyer:

I don't understand the fine details of upgrading, but would it be possible to copy makefile to Makefile during the upgrade?

Of course. But I would rename it because (POSIX) make gives makefile precedence over Makefile and we don't want to cause confusion.

On the other hand, if makefile and Makefile are identical files, it shouldn't matter. It seems just dangerous to delete makefile during a Sage build/upgrade.

Note that on OS X, you can't have files called both "makefile" and "Makefile" in the same directory. Also, "makefile" is not used at all during an upgrade, so it should be safe to rename it.

Can any of you implement this?

It's taken care of by #9433. If we want to implement it here, I think it should be done in the sage-upgrade script, after running "./install" and checking its exit status:

  • sage-upgrade

    diff -r 4047e578febc sage-upgrade
    a b  
    3636    exit 1
    3737fi
    3838
     39cd "$SAGE_ROOT"
     40
     41if [ -f makefile ]; then
     42    mv makefile Makefile
     43fi
     44
     45if [ $? -ne 0 ]; then
     46    echo "Error renaming 'makefile' to 'Makefile'.
     47    exit 1
     48fi

Another option would also check for the existence of 'Makefile' before doing the move:

  • sage-upgrade

    diff -r 4047e578febc sage-upgrade
    a b  
    3636    exit 1
    3737fi
    3838
     39cd "$SAGE_ROOT"
     40
     41if [ -f makefile ]; then
     42    if [ -f Makefile ]; then
     43       mv Makefile Makefile.old
     44    fi
     45    mv makefile Makefile
     46fi
     47
     48if [ $? -ne 0 ]; then
     49    echo "Error renaming 'makefile' to 'Makefile'.
     50    exit 1
     51fi

Opinions either way?

Changed 9 years ago by jdemeyer

Additional scripts patch: rename makefile to Makefile during upgrade

comment:16 Changed 9 years ago by jdemeyer

  • Merged in sage-4.6.1.alpha0 deleted
  • Status changed from needs_work to needs_review

comment:17 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.6.1.alpha1

comment:18 follow-up: Changed 9 years ago by jhpalmieri

  • Authors changed from Jeroen Demeyer to Jeroen Demeyer, John Palmieri

The first two patches (10156_docs.patch and 10156_scripts.patch) get a positive review from me.

This doesn't work, probably my fault. The issue is that the script "sage-upgrade" doesn't get upgraded in the process. So I think moving makefile should happen at the end of the "install" script instead, since this gets downloaded before it gets run. (In particular, upgrading didn't rename "makefile" when I tested it.) Try this patch to "install" instead.

Changed 9 years ago by jhpalmieri

the file spkg/install

Changed 9 years ago by jhpalmieri

patch for 'install'

comment:19 in reply to: ↑ 18 Changed 9 years ago by jhpalmieri

Replying to jhpalmieri:

This doesn't work, probably my fault.

By "this", I meant the third patch, 10156_upgrade.patch.

comment:20 Changed 9 years ago by jhpalmieri

For what it's worth, my change to "install" works for me when upgrading on both Mac OS X (with its odd case-sensitivity) and Solaris. I tested upgrading from 4.5.3 (which had "makefile") and 4.6.1.alpha0 (which had "Makefile").

comment:21 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:22 Changed 9 years ago by jdemeyer

  • Summary changed from Rename makefile to Makefile to Rename top-level makefile to Makefile
  • Work issues copy makefile to Makefile during upgrade deleted

comment:23 Changed 9 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

I successfully updated from sage-4.5.2 on Fedora 13 and the patch did go the extra mile to make sure that it doesn't break upgrades. I'll set it to positive review.

comment:24 Changed 9 years ago by jdemeyer

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

comment:25 follow-up: Changed 9 years ago by drkirkby

It looks to me there are still a few files that refer to makefile. Shall I create a ticket for these:

devel/sage-main/doc/output/latex/en/developer/developer.tex:Another way is to edit the file \code{makefile} in the top level Sage
devel/sage-main/doc/output/latex/en/developer/developer.tex:After saving all changes to \code{makefile}, we can parallel test with the
devel/sage-main/doc/output/latex/en/developer/developer.tex:these command, see the files \code{SAGE\_ROOT/makefile} and
devel/sage-main/doc/output/latex/en/developer/developer.tex:argument \code{-long}. See the file \code{SAGE\_ROOT/makefile} for further
devel/sage-main/doc/output/latex/en/developer/developer.tex:\code{SAGE\_ROOT/makefile} for further details.
devel/sage-main/doc/output/latex/en/developer/developer.tex:the file \code{SAGE\_ROOT/makefile} takes care of the unpacking,
devel/sage-main/doc/output/latex/en/developer/developer.tex:\code{\$SAGE\_ROOT/makefile} before using \code{ptestlong}). See

comment:26 in reply to: ↑ 25 ; follow-up: Changed 9 years ago by jdemeyer

Replying to drkirkby:

It looks to me there are still a few files that refer to makefile. Shall I create a ticket for these:

devel/sage-main/doc/output/latex/en/developer/developer.tex:Another way is to edit the file \code{makefile} in the top level Sage
devel/sage-main/doc/output/latex/en/developer/developer.tex:After saving all changes to \code{makefile}, we can parallel test with the
devel/sage-main/doc/output/latex/en/developer/developer.tex:these command, see the files \code{SAGE\_ROOT/makefile} and
devel/sage-main/doc/output/latex/en/developer/developer.tex:argument \code{-long}. See the file \code{SAGE\_ROOT/makefile} for further
devel/sage-main/doc/output/latex/en/developer/developer.tex:\code{SAGE\_ROOT/makefile} for further details.
devel/sage-main/doc/output/latex/en/developer/developer.tex:the file \code{SAGE\_ROOT/makefile} takes care of the unpacking,
devel/sage-main/doc/output/latex/en/developer/developer.tex:\code{\$SAGE\_ROOT/makefile} before using \code{ptestlong}). See

These are all in the output directory. Could it be that you did not rebuild the pdf documentation after applying these tickets and are finding the old tex files?

comment:27 in reply to: ↑ 26 Changed 9 years ago by drkirkby

Replying to jdemeyer:

These are all in the output directory. Could it be that you did not rebuild the pdf documentation after applying these tickets and are finding the old tex files?

Yes, it is very possible. I'm just going to build Sage again from scratch, as I've messed around with various files. But I very much doubt I built the pdfs, so that explains it.

Dave

Note: See TracTickets for help on using tickets.