Opened 5 years ago

Closed 4 years ago

#19295 closed enhancement (fixed)

Add some dependencies

Reported by: jdemeyer Owned by:
Priority: minor Milestone: sage-7.3
Component: packages: optional Keywords:
Cc: tmonteil, mkoeppe, embray, chapoton, jdemeyer, vbraun, tscrim Merged in:
Authors: Jeroen Demeyer Reviewers: Thierry Monteil, Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 6aef10c (Commits) Commit: 6aef10cce1ff5a056828e5bd0b5a1c09d51c04ad
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

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

Attachments (5)

autotools-20141105.log (69.7 KB) - added by tmonteil 5 years ago.
database_gap-4.7.8.log (4.0 KB) - added by tmonteil 5 years ago.
igraph-0.7.1.log (3.1 KB) - added by tmonteil 5 years ago.
libtheora-1.1.1.log (7.1 KB) - added by tmonteil 5 years ago.
normaliz-2.12.1.p0.log (4.9 KB) - added by tmonteil 5 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/add_some_dependencies

comment:2 Changed 5 years ago by jdemeyer

  • Commit set to 8bd7801f5e9c6fe698f0b7a96aefc8af641f503f
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

8bd7801Add some missing dependencies

comment:3 Changed 5 years ago by jdemeyer

  • Cc tmonteil added
  • Component changed from packages: standard to packages: optional
  • Priority changed from major to minor

comment:4 Changed 5 years ago by git

  • Commit changed from 8bd7801f5e9c6fe698f0b7a96aefc8af641f503f to 48e1694031c401276d8f392acc7a1ae984c52edb

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

48e1694Add a few more dependencies

comment:5 Changed 5 years ago by git

  • Commit changed from 48e1694031c401276d8f392acc7a1ae984c52edb to b0eab92b0babc9180ad114e386a3d0926d9996fb

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

b0eab92Add dependencies for autotools

comment:6 Changed 5 years ago by tmonteil

I am on it, but it takes time.

Changed 5 years ago by tmonteil

Changed 5 years ago by tmonteil

Changed 5 years ago by tmonteil

Changed 5 years ago by tmonteil

Changed 5 years ago by tmonteil

comment:7 Changed 5 years ago by tmonteil

  • Branch changed from u/jdemeyer/add_some_dependencies to u/tmonteil/add_some_dependencies

comment:8 follow-up: Changed 5 years ago by tmonteil

  • Commit changed from b0eab92b0babc9180ad114e386a3d0926d9996fb to ec50fed25a39aa154ad2c3d4a9eab732d801b5a3
  • Reviewers set to Thierry Monteil
  • Status changed from needs_review to needs_work

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

comment:9 in reply to: ↑ 8 Changed 5 years ago by jdemeyer

Replying to 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.

comment:10 Changed 5 years ago by jdemeyer

Does this still need work?

comment:11 Changed 4 years ago by mkoeppe

  • Cc mkoeppe embray chapoton jdemeyer added
  • Milestone changed from sage-6.9 to sage-7.3

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?)

comment:12 Changed 4 years ago by embray

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

comment:13 follow-up: Changed 4 years ago by mkoeppe

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?

comment:14 Changed 4 years ago by mkoeppe

  • Status changed from needs_work to needs_info

comment:15 Changed 4 years ago by mkoeppe

  • Cc vbraun added

comment:16 Changed 4 years ago by vbraun

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

comment:17 Changed 4 years ago by mkoeppe

  • Status changed from needs_info to needs_work

comment:18 Changed 4 years ago by mkoeppe

  • Cc tscrim added

comment:19 in reply to: ↑ 13 Changed 4 years ago by jdemeyer

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

comment:20 Changed 4 years ago by jdemeyer

  • Branch changed from u/tmonteil/add_some_dependencies to u/jdemeyer/add_some_dependencies

comment:21 Changed 4 years ago by jdemeyer

  • Commit changed from ec50fed25a39aa154ad2c3d4a9eab732d801b5a3 to 6aef10cce1ff5a056828e5bd0b5a1c09d51c04ad
  • Status changed from needs_work to needs_review

New commits:

6aef10cUpdate to new style of dependencies

comment:22 Changed 4 years ago by mkoeppe

  • Reviewers changed from Thierry Monteil to Thierry Monteil, Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:23 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/add_some_dependencies to 6aef10cce1ff5a056828e5bd0b5a1c09d51c04ad
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.