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

sage-spkg: Add an option -w for warning only if spkg-check fails #29301

Closed
mkoeppe opened this issue Mar 9, 2020 · 19 comments
Closed

sage-spkg: Add an option -w for warning only if spkg-check fails #29301

mkoeppe opened this issue Mar 9, 2020 · 19 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 9, 2020

In addition, setting SAGE_CHECK=warn has the same effect.

This is useful for automated builds - using make -k SAGE_CHECK=warn we can collect many build and test suite results in one shot.

CC: @dimpase @jhpalmieri

Component: build

Author: Matthias Koeppe

Branch/Commit: f45b28d

Reviewer: Dima Pasechnik, John Palmieri

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

@mkoeppe mkoeppe added this to the sage-9.1 milestone Mar 9, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 12, 2020

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 12, 2020

Commit: 46a220e

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 12, 2020

New commits:

46a220eImplement SAGE_CHECK=warn, sage-spkg -w

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 12, 2020

Author: Matthias Koeppe

@jhpalmieri
Copy link
Member

comment:4

If you set SAGE_CHECK=warn, it doesn't behave just like passing the -w flag to sage-spkg: the latter sets SAGE_CHECK_PACKAGES=x, while the former does not. Therefore the default setting SAGE_CHECK_PACKAGES='!python2,!python3' gets used with the former.

What is the intent?

(Aside: should flags like -c and now -w also have corresponding Makefile targets? We shouldn't have to set environment variables to do this.)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 12, 2020

comment:5

Replying to @jhpalmieri:

If you set SAGE_CHECK=warn, it doesn't behave just like passing the -w flag to sage-spkg: the latter sets SAGE_CHECK_PACKAGES=x, while the former does not. Therefore the default setting SAGE_CHECK_PACKAGES='!python2,!python3' gets used with the former.

What is the intent?

I set SAGE_CHECK=warn to enable this behavior as a default for all packages. I don't see why the default SAGE_CHECK_PACKAGES='!python2,!python3' should not apply. In my test scripts, I set SAGE_CHECK_PACKAGES to more exclusions (for example, ppl has a testsuite that takes very long.)

(Aside: should flags like -c and now -w also have corresponding Makefile targets? We shouldn't have to set environment variables to do this.)

Makefile targets, I don't think. We could make it configure options though. (In some follow-up ticket.)

@mkoeppe

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:7

I think that if SAGE_CHECK=warn and if SAGE_CHECK_PACKAGES is empty, then everything should be tested, even Python. (The reason for skipping Python is not because it takes a long time, but rather to avoid make stopping when Python's tests fail.) Otherwise, do I need to set SAGE_CHECK_PACKAGES to some artificial setting to test all packages, including Python?

If SAGE_CHECK_PACKAGES is not empty, then setting SAGE_CHECK=warn should not affect that.

Re Makefile targets vs. configure: it feels more standard to have make check or similar, rather than have it be a configure option. Not to be done on this ticket, either way.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 12, 2020

Changed commit from 46a220e to f45b28d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 12, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

f45b28dIf SAGE_CHECK=warn, do not exclude python by default via SAGE_CHECK_PACKAGES

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 12, 2020

comment:9

Is this doing what you have in mind?

@jhpalmieri
Copy link
Member

comment:10

At first glance it looks good, but I have to test it. Ideally if I run make with SAGE_CHECK=warn, then there would be summary information at the end. ("The build succeeded but the following packages failed their test suites: ...") Any ideas how to do that?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 13, 2020

comment:11

Well, the code for the summary "The following package(s) may have failed to build..." is currently only run on failure.
I suppose we could run it on success too if SAGE_CHECK=warn...

@dimpase
Copy link
Member

dimpase commented Mar 24, 2020

comment:12

OK, seems to work

@dimpase
Copy link
Member

dimpase commented Mar 24, 2020

Reviewer: Dima Pasechnik, John Palmieri

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

comment:13

Thanks! Further refinements can be done on a follow-up ticket.

@jhpalmieri
Copy link
Member

comment:14

Followup at #29402.

@vbraun
Copy link
Member

vbraun commented Mar 29, 2020

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

4 participants