Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#11701 closed enhancement (wontfix)

Add spkg-check to Mercurial

Reported by: kcrisman Owned by: tbd
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: packages: standard Keywords:
Cc: jason, ryan, kini, jhpalmieri, leif Merged in:
Authors: Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #10594 Stopgaps:

Description

Mercurial was recently upgraded at #10594. It was mentioned there that it has self-tests. We should enable those to be checked with SAGE_CHECK and an spkg-check script.

Change History (19)

comment:1 Changed 8 years ago by leif

  • Cc leif added

comment:2 follow-up: Changed 8 years ago by leif

For the record: The new Mercurial spkg didn't make it into Sage 4.7.1.

comment:3 in reply to: ↑ 2 Changed 8 years ago by kcrisman

  • Dependencies set to #10594

Replying to leif:

For the record: The new Mercurial spkg didn't make it into Sage 4.7.1.

Correct, and I didn't mean to imply that. In theory, this wouldn't even necessarily depend on that, if the invocation for the self-tests are the same in both versions. I'll put that in dependencies nonetheless.

comment:4 follow-ups: Changed 8 years ago by leif

P.S.:

We might actually (always) run the test suite from spkg-install instead (before we install Mercurial), since it is a central component and -- more important -- installing it, afterwards realizing some tests fail, doesn't make much sense.

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

We might actually (always) run the test suite from spkg-install instead (before we install Mercurial), since it is a central component and -- more important -- installing it, afterwards realizing some tests fail, doesn't make much sense.

I think we do this for a few packages, right? Either one would be a fine solution to this ticket.

comment:6 in reply to: ↑ 4 Changed 8 years ago by leif

Replying to leif:

We might actually (always) run the test suite from spkg-install instead (before we install Mercurial), since it is a central component and -- more important -- installing it, afterwards realizing some tests fail, doesn't make much sense.

... since the user would probably end up with a broken, perhaps broadly unusable Sage installation.

(Of course he/she should still be able to manually reinstall a previous Mercurial version though, but I think avoiding the need for that is desirable.)

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

Replying to kcrisman:

I think we do this for a few packages, right? Either one would be a fine solution to this ticket.

The overall design w.r.t. SAGE_CHECK and spkg-check is a bit suboptimal. The only spkg where we unconditionally run the test suite from spkg-install I'm currently aware of is MPFR; there might be very few others, since we changed that in a couple of spkgs a while ago.

(I'm not sure what you mean by either one; just adding an spkg-check vs. running the test suite from spkg-install?)

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

Replying to leif:

Replying to kcrisman:

I think we do this for a few packages, right? Either one would be a fine solution to this ticket.

The overall design w.r.t. SAGE_CHECK and spkg-check is a bit suboptimal. The only spkg where we unconditionally run the test suite from spkg-install I'm currently aware of is MPFR; there might be very few others, since we changed that in a couple of spkgs a while ago.

(I'm not sure what you mean by either one; just adding an spkg-check vs. running the test suite from spkg-install?)

Yes, either of those would be fine, unless the spkg-install version added significantly to the Sage build time.

comment:9 Changed 8 years ago by jhpalmieri

From the src directory, I ran "make" and then "time make tests". On an OS X box:

# Ran 407 tests, 35 skipped, 32 failed.
make: *** [tests] Error 1

real	22m55.609s
user	10m16.167s
sys	8m57.330s

On sage.math:

# Ran 407 tests, 25 skipped, 2 failed.
make: *** [tests] Error 1

real	13m28.549s
user	7m48.570s
sys	2m27.840s

I haven't looked at the output to try to figure out what the failures were, but in any case, this testing process takes much longer than the build process. So I think that we should put this into an spkg-check file, not into spkg-install. It would be then be annoying to have another spkg (along with python) which is known to fail self-tests on standard platforms.

comment:10 follow-up: Changed 8 years ago by kcrisman

Agreed - definitely let's not make this standard.

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

Replying to kcrisman:

Agreed - definitely let's not make this standard.

... or modify the tests run "by default" by patching the Makefile; this of course requires sorting out which tests fail where (and probably why), so room for a follow-up I guess.

comment:12 in reply to: ↑ 11 Changed 8 years ago by jhpalmieri

Replying to leif:

Replying to kcrisman:

Agreed - definitely let's not make this standard.

... or modify the tests run "by default" by patching the Makefile; this of course requires sorting out which tests fail where (and probably why), so room for a follow-up I guess.

My point for making it non-standard wasn't the failures, it was the time required. To install the spkg takes under 5 seconds on my OS X box, whereas running self-tests takes 23 minutes. We can't add this much time to the default Sage installation, especially since Mercurial seems to function just fine -- the failures don't seem to be too important. In my opinion, if we want to make some tests standard, we have to just run a few absolutely crucial ones so we don't add too much time; the full test suite is way too large to include every time.

If we can identify which tests fail and why, and if those particular tests aren't important for our uses, then I agree that (probably on a follow-up) we should document those failures, so people know they may not be getting a fully functional Mercurial installation, and skip those tests. (It would be nice to do the same for the Python spkg...)

comment:13 Changed 8 years ago by jhpalmieri

Actually, rather than editing the Makefile, we can do make test-foo for various values of foo.

comment:14 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:15 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:16 Changed 6 years ago by jhpalmieri

  • Milestone changed from sage-6.2 to sage-duplicate/invalid/wontfix
  • Reviewers set to John Palmieri
  • Status changed from new to needs_review

Let's close this, since we no longer distribute Mercurial.

comment:17 Changed 6 years ago by jhpalmieri

  • Status changed from needs_review to positive_review

comment:18 Changed 6 years ago by vbraun

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

comment:19 Changed 6 years ago by vbraun

  • Resolution changed from fixed to wontfix
Note: See TracTickets for help on using tickets.