Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically detect and test optional dependencies #13540

Closed
robertwb opened this issue Sep 27, 2012 · 31 comments
Closed

Automatically detect and test optional dependencies #13540

robertwb opened this issue Sep 27, 2012 · 31 comments

Comments

@robertwb
Copy link
Contributor

I'd deferred actually fixing the broken doctests to #13884, #13885, and #2120 so we can start using this (and not have everything optional broken).


Superseded / fixed by #18558, #20182.

CC: @jhpalmieri @gvol @slel

Component: doctest framework

Reviewer: Kwankyu Lee, ​Samuel Lelièvre

Issue created by migration from https://trac.sagemath.org/ticket/13540

@jdemeyer
Copy link
Contributor

comment:1

Checking just for the existence of gcc is insufficient, you would have a lot of false positives with missing/broken assemblers/linkers... You actually need to compile something, preferably a shared library.

@jdemeyer
Copy link
Contributor

Author: Robert Bradshaw

@jdemeyer
Copy link
Contributor

comment:2

For the internet test: could you use www.sagemath.org instead of 173.194.33.3. In any case, an IP address is bad because

  1. you don't test DNS, which is required.
  2. it can change.

@jdemeyer
Copy link
Contributor

comment:3

Also please add a check for g++ and gfortran.

@jdemeyer
Copy link
Contributor

comment:4

Instead of relying on which, simply execute the command and catch OSError.

@jdemeyer
Copy link
Contributor

comment:5

Command lines for gcc, g++, gfortran compiling a shared library from /dev/null:

gcc -shared -fpic -x c /dev/null -o /dev/null
g++ -shared -fpic -x c++ /dev/null -o /dev/null
gfortran -cpp -ffree-form -shared -fpic -x f95 /dev/null -o /dev/null

I have no idea how to do any of this with other compilers such as clang.

@jdemeyer
Copy link
Contributor

comment:6

I think it would be good to implement either caching or lazy evaluation (only check when actually encountering # optional - foo). Otherwise, somebody with a slow internet connection could see a substantial time added to every doctest run.

@jdemeyer
Copy link
Contributor

comment:7

For programs like magma, does Sage require a specific version? If so, you should check for that version.

@robertwb
Copy link
Contributor Author

comment:8

I would be surprised if many people that have gcc have a broken gcc, but it couldn't hurt to check (and g++, etc.) Same with internet--it's rare for people to be connected but not be able to resolve DNS. (If this is lazy, latency less of an issue.)

Good points. I'll refactor this to make things lazy and otherwise improve the patch.

@robertwb
Copy link
Contributor Author

robertwb commented Oct 9, 2012

comment:11

Attachment: 13540-optional-tests.2.patch.gz

Apply only 13540-optional-tests.2.patch

@robertwb
Copy link
Contributor Author

comment:12

Apply 13540-optional-tests.2.patch to sage-local repo.

@vbraun
Copy link
Member

vbraun commented Dec 20, 2012

comment:13

The patch is great but a great number of the # optional - internet doctests fail because they haven't been doctested in a while... Any opinion on how to proceeed?

@robertwb
Copy link
Contributor Author

Attachment: 13540-disable-broken.patch.gz

@robertwb

This comment has been minimized.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Dec 30, 2012

comment:15

IMHO we shouldn't hardcode the names of the executables (gcc, g++, gfortran etc.), but respect for example CC, CXX, FC/F77/F95.

(Also wonder whether "# optional - gcc" really means GCC, or some C compiler, perhaps at least capable of compiling Cython-generated code.)

@nexttime
Copy link
Mannequin

nexttime mannequin commented Dec 30, 2012

comment:16

Replying to @nexttime:

IMHO we shouldn't hardcode the names of the executables (gcc, g++, gfortran etc.), but respect for example CC, CXX, FC/F77/F95.

As an alternative, we might put their settings "during build" (whatever that means) into dummy/wrapper scripts in $SAGE_LOCAL/bin/. (I don't really have an opion on that, but tend to be against such. It might be convenient to "ordinary" users, or perhaps for automated testing, but annoying or confusing to developers.)

@robertwb
Copy link
Contributor Author

comment:17

For all intents and purposes, at the moment, gcc really means gcc. And building a Cython extension with a different compiler than Python was compiled with (and the relevant libraries, for C++ in particular) gets really messy.

In any case, the point of this ticket is to introduce a framework that lets us test optional doctests automatically, as they the current setup means they're never tested and so quickly broken. We could do something more sophisticated with CC, etc. but gcc, at the very least, provides a common enough dependency to actually exercise this code for most (probably all) developers (and it's not near as brittle as others).

@nexttime
Copy link
Mannequin

nexttime mannequin commented Dec 30, 2012

comment:18

Replying to @robertwb:

For all intents and purposes, at the moment, gcc really means gcc. And building a Cython extension with a different compiler than Python was compiled with (and the relevant libraries, for C++ in particular) gets really messy.

We might then even use Python's / distutils' settings for testing tool availability (although there's no variable for the Fortran compiler AFAIK). Not sure what to do with bdists (in case they don't contain a pre-built GCC as well).

In any case, the point of this ticket is to introduce a framework that lets us test optional doctests automatically, as they the current setup means they're never tested and so quickly broken. We could do something more sophisticated with CC, etc. but gcc, at the very least, provides a common enough dependency to actually exercise this code for most (probably all) developers (and it's not near as brittle as others).

Yes. Nevertheless, I don't think I'm the only one that almost always sets CC, CXX etc., in order to use different versions of GCC.

Support for other compilers (e.g. clang), probably also flags to pass, is certainly out of the scope of this ticket. (It would be convenient to have something like config.status, i.e., a mechanism for storing and restoring the build environment, and/or also a ".sagerc" local / specific to the Sage installation, one overriding the other...)

@jdemeyer
Copy link
Contributor

comment:19

How is this supposed to interact with the --optional and --only-optional command line options? I have a hard time understanding the new code.

@jdemeyer
Copy link
Contributor

comment:21

This would obviously need to be rebased to #12415.

@jdemeyer jdemeyer added this to the sage-5.12 milestone Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@nexttime
Copy link
Mannequin

nexttime mannequin commented May 17, 2014

comment:26

Magma doctest failures now get fixed at #16322.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 31, 2015

comment:28

Hello everybody,

A branch that does that has been created at #18558 (needs review)

Nathann

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 27, 2016

comment:29

This is now "duplicate/wontfix" as the replacement #20182 is now merged.

@kwankyu kwankyu removed this from the sage-6.4 milestone Apr 27, 2016
@slel
Copy link
Member

slel commented Aug 29, 2016

comment:30

Is this ticket solved by the other tickets mentioned in the last comments?
Does any comment in the discussion beg for a follow-up ticket?

@slel slel changed the title Automatically detect and test optional dependencies. Automatically detect and test optional dependencies Aug 29, 2016
@kwankyu
Copy link
Collaborator

kwankyu commented Aug 29, 2016

comment:31

Replying to @slel:

Is this ticket solved by the other tickets mentioned in the last comments?

Yes.

Does any comment in the discussion beg for a follow-up ticket?

I don't think so. Most of the comments in this discussion are 4 years old, and so perhaps invalid now.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 29, 2016

comment:33

4-year old => invalid

ROFL.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 29, 2016

Reviewer: Kwankyu Lee, ​Samuel Lelièvre

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 29, 2016

Changed author from Robert Bradshaw to none

@nexttime

This comment has been minimized.

@embray
Copy link
Contributor

embray commented Aug 30, 2016

comment:35

Determined to be invalid/duplicate/wontfix (closing as "wontfix" as a catch-all resolution).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants