Opened 8 years ago

Closed 8 years ago

#10970 closed defect (duplicate)

Do not generate pipestatus from spkg/install

Reported by: vbraun Owned by: GeorgSWeber
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords: sd32
Cc: jdemeyer, leif Merged in:
Authors: Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

The $SAGE_ROOT/spkg/install has a here document that creates pipestatus. With the sage root repository, this is no longer needed as pipestatus is tracked by the repository. The patch removes it from $SAGE_ROOT/spkg/install.


Patch moved to #11073.

Attachments (2)

trac_10970_do_not_generate_pipestatus.patch (3.1 KB) - added by vbraun 8 years ago.
Patch to SAGE_ROOT repo
trac_10970-error-msg.patch (1.3 KB) - added by jhpalmieri 8 years ago.
apply on top of other patch

Download all attachments as: .zip

Change History (34)

Changed 8 years ago by vbraun

Patch to SAGE_ROOT repo

comment:1 Changed 8 years ago by vbraun

  • Cc jeroen added
  • Status changed from new to needs_review

comment:2 Changed 8 years ago by mariah

  • Reviewers set to Mariah Lenox
  • Status changed from needs_review to positive_review

Patch applied, then patched file moved to new source directory. 'make testlong' passed all tests. Positive review.

comment:3 follow-up: Changed 8 years ago by vbraun

  • Status changed from positive_review to needs_info

I've recently played around with automatic testing upgrades and I think this might break upgrades. Its really a bigger issue, the sage_root repository is only upgraded rather late in the upgrade process. I think this needs to be fixed first. I will do some more testing when I have time.

I'll set this ticket to "needs info" until I (or somebody else) tests upgrading.

comment:4 follow-up: Changed 8 years ago by jhpalmieri

My guess is that if this causes problems with upgrading, it should only happen when upgrading from versions of Sage which don't already include the file "pipestatus", which would mean some version between 4.4.4 (which doesn't have it) and 4.5.3 (which does). I haven't actually tested anything, though.

Maybe the root repository should be upgraded earlier. In an install from scratch, it has to be installed somewhat late because it depends on mercurial, but when upgrading, mercurial should already be present. This would require altering the upgrade process so that it doesn't just run spkg/install. We could install the root repo by hand (in sage-upgrade) before running spkg/install.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 8 years ago by vbraun

Replying to jhpalmieri:

Maybe the root repository should be upgraded earlier. In an install from scratch, it has to be installed somewhat late because it depends on mercurial, but when upgrading, mercurial should already be present.

Those were exactly my thoughts, too.

I was mostly motivated by #11073. We can't stuff all files in the spkg/base repo into a here document :-)

Do we have an official policy on how old a Sage version should still be able to upgrade?

comment:6 Changed 8 years ago by leif

  • Cc jdemeyer leif added; jeroen removed

comment:7 in reply to: ↑ 5 Changed 8 years ago by was

Replying to vbraun:

Do we have an official policy on how old a Sage version should still be able to upgrade?

I declare "1 year" as the official policy, since that's the official policy for deprecation. I think it is a reasonable amount of time.

Since sage-4.5.3 had pipestatus, and was released 2010-09-08 and the next sage release won't be for two weeks (or so), it is fine to deprecate upgrading from pre sage-4.5.3 releases.

-- William

comment:8 Changed 8 years ago by jdemeyer

  • Status changed from needs_info to needs_review
  • Summary changed from Remove pipestatus from spkg/install to Do not generate pipestatus from spkg/install

pipestatus exists since sage-4.5, released 16 july 2010.

comment:9 follow-ups: Changed 8 years ago by was

I mentioned this to people at Sage Days and they point out that "sage -upgrade" rarely works anyways, so we shouldn't be be too worried about breaking "sage -upgrade".

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

Replying to was:

I mentioned this to people at Sage Days and they point out that "sage -upgrade" rarely works anyways[citation needed], so we shouldn't be be too worried about breaking "sage -upgrade".

We do test upgrading on various systems from various Sage versions and we almost never have problems.

Anothe question is: is upgrading really used? I mean, do normal Sage users sometimes do sage -upgrade? If not, then why do we support upgrading?

comment:11 in reply to: ↑ 10 Changed 8 years ago by was

  1. Jdemeyer -- thank you for doing testing. I greatly appreciate it. This would be a great topic for our "testing sage days".

Replying to jdemeyer:

Anothe question is: is upgrading really used? I mean, do normal Sage users sometimes do sage -upgrade? If not, then why do we support upgrading?

*I* use sage -upgrade a lot on my servers that have lots of optional packages installed.

comment:12 Changed 8 years ago by was

  • Status changed from needs_review to positive_review

I did some testing of this... and I say: positive review.

comment:13 Changed 8 years ago by was

  • Keywords sd32 added

comment:14 in reply to: ↑ 9 Changed 8 years ago by leif

Replying to was:

I mentioned this to people at Sage Days and they point out that "sage -upgrade" rarely works anyways, so we shouldn't be be too worried about breaking "sage -upgrade".

We should be worried that (or why they feel) upgrading doesn't work for them though. I guess they just didn't try since last year (when it started working better). ;-)


However, I see no real point in (or any compelling reason for) removing pipestatus' creation.

If we do so, despite of that it [just] breaks upgrading from older releases, spkg/install should give an appropriate error message in case pipestatus isn't already present. Unfortunately this would happen after dozens of spkgs to upgrade have been downloaded, which is a bit odd.

If at all, we IMHO should remove it in some "major" release, like e.g. Sage 5.0, or perhaps Sage 4.8, since people are probably less likely to upgrade to such a version (with -upgrade), or at least more likely to read its release notes in advance.

comment:15 Changed 8 years ago by leif

P.S.: The current patch does not even give an [early, intentional] error message in case pipestatus is missing.

I also think supporting sage -upgrade is valuable; note that it still issues a scary warning message we should IMHO change.

comment:16 follow-up: Changed 8 years ago by was

leif -- just out of curiosity, do you use gentoo?

comment:17 in reply to: ↑ 16 ; follow-up: Changed 8 years ago by leif

Replying to was:

leif -- just out of curiosity, do you use gentoo?

Nope...

comment:18 in reply to: ↑ 17 Changed 8 years ago by leif

Replying to leif:

Replying to was:

leif -- just out of curiosity, do you use gentoo?

Nope...

Should I? (And I'm not aware Volker does; I also didn't hear François complaining about pipestatus' creation, but maybe I just missed it.)

comment:19 Changed 8 years ago by leif

  • Description modified (diff)

I still think removing pipestatus for no reason at this point is simply superfluous.

The patch should IMHO at least test for pipestatus's presence, and issue an appropriate error message (with instructions how to fix this) if it doesn't exist; though quite late in the upgrade process, i.e. after all updated packages have been downloaded, of course.

P.S.: We could even attempt to download it (with e.g. curl or wget) from the script. Or install the root repository "manually" in advance on upgrading, of course from the updated spkg/install, since the other scripts are those of the old version.

comment:20 Changed 8 years ago by leif

  • Description modified (diff)

comment:21 follow-up: Changed 8 years ago by jhpalmieri

In addition to leif's comments, note that the pipestatus script says

# IMPORTANT: if you edit this file, be sure to also edit spkg/install
#            where this file is created for upgrading.

This comment should probably be deleted.

comment:22 in reply to: ↑ 21 Changed 8 years ago by leif

  • Status changed from positive_review to needs_info

Replying to jhpalmieri:

In addition to leif's comments, note that the pipestatus script says

# IMPORTANT: if you edit this file, be sure to also edit spkg/install
#            where this file is created for upgrading.

This comment should probably be deleted.

Do you agree that this ticket needs works, w.r.t. the comment, and -- IMHO more important -- the lack of an error check + message on how to fix this?

comment:23 Changed 8 years ago by vbraun

I think the elephant in the room is #11073: remove the spkg/base repo, add a mechanism that adds the absolutely necessary files to the tarball (or directly downloads them for upgrades) without depending on bzip2/mercurial/.... Then pipestatus would just one of those "base" scripts and everything would be taken care of automatically. So perhaps we should make this ticket just contingent upon #11073 and focus our energy there.

comment:24 Changed 8 years ago by jhpalmieri

  • Description modified (diff)

I'm attaching a referee patch printing an error message if pipestatus is missing. We could also patch the scripts repo:

  • sage-update

    diff --git a/sage-update b/sage-update
    a b def do_update(): 
    400400    os.chdir(SPKG_ROOT)
    401401    download_file("install")
    402402    os.system('chmod +x install')
     403    download_file("pipestatus")
     404    os.system('chmod +x pipestatus')
    403405    os.chdir(os.path.join(SPKG_ROOT, "standard"))
    404406    download_file("standard/deps")
    405407    download_file("standard/newest_version")
    def do_update(): 
    408410    os.chdir(SAGE_ROOT)
    409411    if root_repo_intact:
    410412        for file in ['spkg/install',
     413                     'spkg/pipestatus',
    411414                     'spkg/standard/deps',
    412415                     'spkg/standard/newest_version']:
    413416            subprocess.call('./sage -hg commit %s -m ' % file +

but I'm not sure it's worth it: pipestatus is not a script which should change a lot, so if you have an out-of-date version, it's probably good enough, and it will get replaced during the upgrade process and used the next time around.

Changed 8 years ago by jhpalmieri

apply on top of other patch

comment:25 in reply to: ↑ 3 Changed 8 years ago by jdemeyer

  • Status changed from needs_info to needs_review

Replying to vbraun:

I've recently played around with automatic testing upgrades and I think this might break upgrades. Its really a bigger issue, the sage_root repository is only upgraded rather late in the upgrade process. I think this needs to be fixed first. I will do some more testing when I have time.

Any results? Which version failed to upgrade?

Concerning #11073: I don't see why it should affect this ticket. I think we should be able to merge this ticket now (if it doesn't break upgrades).

comment:26 Changed 8 years ago by jdemeyer

  • Reviewers changed from Mariah Lenox to Jeroen Demeyer, Mariah Lenox

comment:27 Changed 8 years ago by jdemeyer

The only annoying thing here is that spkg/install is executed twice: if the upgrade fails the first time, it is run again a second time. There is no way for spkg/install to say "this is really a failure, no point trying again". That would require a change to local/bin/sage-upgrade (of the old Sage version!).

comment:28 Changed 8 years ago by jdemeyer

  • Upgrading from 4.4.4 failed (with the appropriate error message)
  • Upgrading from 4.5 and 4.6 also failed, but while installing boehm_gc-7.2.alpha6.p0. This looks unlikely to be related to this ticket:
    /bin/bash ./libtool --tag=CC   --mode=link gcc -fexceptions -g -O2  -version-info 1:3:0 -no-undefined  -o libgc.la -rpath /mnt/usb1/scratc
    h/jdemeyer/sage-4.6/local/lib allchblk.lo alloc.lo blacklst.lo checksums.lo dbg_mlc.lo dyn_load.lo finalize.lo gc_dlopen.lo gcj_mlc.lo hea
    ders.lo malloc.lo mallocx.lo mark.lo mark_rts.lo misc.lo new_hblk.lo obj_map.lo os_dep.lo pcr_interface.lo ptr_chck.lo real_malloc.lo recl
    aim.lo specific.lo stubborn.lo typd_mlc.lo backgraph.lo thread_local_alloc.lo pthread_start.lo pthread_support.lo pthread_stop_world.lo
    atomic_ops.lo mach_dep.lo -lpthread -ldl
    libtool: link: gcc -shared  -fPIC -DPIC  .libs/allchblk.o .libs/alloc.o .libs/blacklst.o .libs/checksums.o .libs/dbg_mlc.o .libs/dyn_load.
    o .libs/finalize.o .libs/gc_dlopen.o .libs/gcj_mlc.o .libs/headers.o .libs/malloc.o .libs/mallocx.o .libs/mark.o .libs/mark_rts.o .libs/mi
    sc.o .libs/new_hblk.o .libs/obj_map.o .libs/os_dep.o .libs/pcr_interface.o .libs/ptr_chck.o .libs/real_malloc.o .libs/reclaim.o .libs/spec
    ific.o .libs/stubborn.o .libs/typd_mlc.o .libs/backgraph.o .libs/thread_local_alloc.o .libs/pthread_start.o .libs/pthread_support.o .libs/
    pthread_stop_world.o .libs/atomic_ops.o .libs/mach_dep.o   -lpthread -ldl  -O2   -Wl,-soname -Wl,libgc.so.1 -o .libs/libgc.so.1.0.3
    libtool: link: (cd ".libs" && rm "libgc.so.1" && ln -s "libgc.so.1.0.3" "libgc.so.1")
    rm: cannot remove `libgc.so.1': No such file or directory
    make[2]: *** [libgc.la] Error 1
    make[2]: Leaving directory `/mnt/usb1/scratch/jdemeyer/sage-4.6/spkg/build/boehm_gc-7.2.alpha6.p0/src'
    make[1]: *** [all-recursive] Error 1
    make[1]: Leaving directory `/mnt/usb1/scratch/jdemeyer/sage-4.6/spkg/build/boehm_gc-7.2.alpha6.p0/src'
    Error building BoehmGC.
    
    real    0m38.188s
    user    0m13.470s
    sys     0m5.540s
    sage: An error occurred while installing boehm_gc-7.2.alpha6.p0
    Please email sage-devel http://groups.google.com/group/sage-devel
    explaining the problem and send the relevant part of
    of /mnt/usb1/scratch/jdemeyer/sage-4.6/install.log.  Describe your computer, operating system, etc.
    If you want to try to fix the problem yourself, *don't* just cd to
    /mnt/usb1/scratch/jdemeyer/sage-4.6/spkg/build/boehm_gc-7.2.alpha6.p0 and type 'make check' or whatever is appropriate.
    Instead, the following commands setup all environment variables
    correctly and load a subshell for you to debug the error:
    (cd '/mnt/usb1/scratch/jdemeyer/sage-4.6/spkg/build/boehm_gc-7.2.alpha6.p0' && '/mnt/usb1/scratch/jdemeyer/sage-4.6/sage' -sh)
    When you are done debugging, you can type "exit" to leave the
    subshell.
    make: *** [installed/boehm_gc-7.2.alpha6.p0] Error 1
    

comment:29 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-4.8 to sage-5.0

comment:30 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.0 to sage-duplicate/invalid/wontfix
  • Status changed from needs_review to positive_review

Since this ticket fits well with #11073, I will merge it in #11073.

comment:31 Changed 8 years ago by jdemeyer

  • Authors Volker Braun deleted
  • Description modified (diff)
  • Reviewers changed from Jeroen Demeyer, Mariah Lenox to Jeroen Demeyer

comment:32 Changed 8 years ago by jdemeyer

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.