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

Add some dependencies #19295

Closed
jdemeyer opened this issue Sep 25, 2015 · 29 comments
Closed

Add some dependencies #19295

jdemeyer opened this issue Sep 25, 2015 · 29 comments

Comments

@jdemeyer
Copy link

A more or less arbitrary list of packages for which I added a dependencies file.

CC: @sagetrac-tmonteil @mkoeppe @embray @fchapoton @jdemeyer @vbraun @tscrim

Component: packages: optional

Author: Jeroen Demeyer

Branch/Commit: 6aef10c

Reviewer: Thierry Monteil, Matthias Koeppe

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

@jdemeyer
Copy link
Author

Branch: u/jdemeyer/add_some_dependencies

@jdemeyer
Copy link
Author

Commit: 8bd7801

@jdemeyer
Copy link
Author

New commits:

8bd7801Add some missing dependencies

@jdemeyer

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 27, 2015

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

48e1694Add a few more dependencies

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 27, 2015

Changed commit from 8bd7801 to 48e1694

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 28, 2015

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

b0eab92Add dependencies for autotools

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 28, 2015

Changed commit from 48e1694 to b0eab92

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Sep 30, 2015

comment:6

I am on it, but it takes time.

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Oct 1, 2015

Attachment: autotools-20141105.log

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Oct 1, 2015

Attachment: database_gap-4.7.8.log

Attachment: igraph-0.7.1.log

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Oct 1, 2015

Attachment: libtheora-1.1.1.log

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Oct 1, 2015

Attachment: normaliz-2.12.1.p0.log

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Oct 1, 2015

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Oct 1, 2015

Reviewer: Thierry Monteil

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Oct 1, 2015

comment:8

Here was my testing protocol:

For each package pkg, start from a Sage with only

    make distclean
    make base

Then do make $pkg and see what happens.

The things that made me crazy is that if some package do not have a dependencies file, then the build system starts to build every standard package, i didn't underdtood that... Because of that "feature", we have to ensure that all dependencies of a package that is tested has a (possibly trivial) dependency file, recursively, or we are not testing much. So, to be tested well, this ticket should depend on #19261.

Now about the packages involved in the ticket, i had problems with the following:

  • autotools still creates a bug, but i do not know what is missing, see the attached log.

  • database_gap: because of the last line in spkg-install script, sage -c "gap_reset_workspace()", it should depend on a working Sage installation. So i added SAGERUNTIME as a dependency. It is overkill ?

  • igraph requires the libxml2 (which we are not shipping), see the attached log. Perhaps we should package it, or perhaps move it to expermiental ? I added some hints on SPKG.txt. Moreover, from https://github.com/igraph/igraph/blob/master/debian/control it seems that igraph would be happy with blas/lapack and glpk, so perhaps should we also add atlas and glpk as dependencies ?

  • libtheora depends on libogg, see the attached log. I updated the dependency file.

  • normaliz depends on boost_cropped, see the attached log. I updated the dependency file. Also, Singular appears in the dependencies, but i am not sure it is the case, to my understanding of the documentation, i am not sure, but it seems that Singular can use Normaliz, but the converse. Shall we remove singular from normaliz dependencies ?

  • i added a # no dependencies for configure.

Also, only configure is a "base" package, while "make base" installs:

base: $(INST)/$(BZIP2) $(INST)/$(PATCH) $(INST)/$(PKGCONF)

Shall those 3 packages have "base" priority too ?


New commits:

ec50fed#19295 review

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Oct 1, 2015

Changed commit from b0eab92 to ec50fed

@jdemeyer
Copy link
Author

jdemeyer commented Oct 2, 2015

comment:9

Replying to @sagetrac-tmonteil:

The things that made me crazy is that if some package do not have a dependencies file, then the build system starts to build every standard package, i didn't underdtood that...

That's the default for optional packages. It was chosen because it's reasonably safe. But after these tickets have been dealt with, we should probably make dependencies mandatory.

  • autotools still creates a bug, but i do not know what is missing, see the attached log.

Are you sure that you tested this correctly? Because it seems from the log that ncurses wasn't installed. Note that I added the autotools dependency in an additional commit.

  • database_gap: because of the last line in spkg-install script, sage -c "gap_reset_workspace()", it should depend on a working Sage installation. So i added SAGERUNTIME as a dependency. It is overkill ?

No overkill, it is correct.

  • igraph requires the libxml2 (which we are not shipping), see the attached log.
    Perhaps we should package it, or perhaps move it to expermiental ?

Let's leave this for now.

it seems that igraph would be happy with blas/lapack and glpk, so perhaps should we also add atlas and glpk as dependencies ?

Well, igraph doesn't use them even if they are present. While igraph might be able to use those libraries, it doesn't do so by default. So no extra dependencies are needed.

  • libtheora depends on libogg, see the attached log. I updated the dependency file.

Good.

  • normaliz depends on boost_cropped, see the attached log. I updated the dependency file.

Good.

Shall we remove singular from normaliz dependencies ?

The spkg-install file for normaliz really installs things specifically for Singular, so I think we need the dependency.

  • i added a # no dependencies for configure.

Well, configure isn't really a package... If you really want a dependencies file, it should perhaps say something like # not a real package.

base: $(INST)/$(BZIP2) $(INST)/$(PATCH) $(INST)/$(PKGCONF)

Shall those 3 packages have "base" priority too ?

No. Like I said, configure isn't a package, but these 3 are real packages.

@jdemeyer
Copy link
Author

comment:10

Does this still need work?

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 26, 2016

comment:11

Part of this (for latte_int, 4ti2) was duplicated in #20748.
What's up with this bigger patch?
(And what's up with all these tickets without followup?)

@mkoeppe mkoeppe modified the milestones: sage-6.9, sage-7.3 Jun 26, 2016
@embray
Copy link
Contributor

embray commented Jun 27, 2016

comment:12

Well, I guess it will have to be rebased anyhow.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 27, 2016

comment:13

This patch adds lines of the form:

 $(INST)/$(SAGE_MP_LIBRARY) $(INST)/$(NTL) $(INST)/$(CDDLIB)

whereas #20748 added:

 $(MP_LIBRARY) ntl 4ti2 cddlib

What is the preferred way of writing these dependencies?
Is this documented?

@vbraun
Copy link
Member

vbraun commented Jun 29, 2016

comment:16

This is a recent change, the new syntax is $(MP_LIBRARY) ntl 4ti2 cddlib

@jdemeyer
Copy link
Author

comment:19

Replying to @mkoeppe:

What is the preferred way of writing these dependencies?
Is this documented?

http://doc.sagemath.org/html/en/developer/packaging.html#package-dependencies

@jdemeyer
Copy link
Author

@jdemeyer
Copy link
Author

Changed commit from ec50fed to 6aef10c

@jdemeyer
Copy link
Author

New commits:

6aef10cUpdate to new style of dependencies

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 15, 2016

Changed reviewer from Thierry Monteil to Thierry Monteil, Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented Jul 16, 2016

Changed branch from u/jdemeyer/add_some_dependencies to 6aef10c

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