Opened 4 years ago

Closed 4 years ago

#20140 closed enhancement (fixed)

dependencies: use "foo" instead of "$(INST)/$(FOO)"

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-7.1
Component: build Keywords:
Cc: SimonKing, vbraun, fbissey Merged in:
Authors: Jeroen Demeyer Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: 383b1b4 (Commits) Commit: 383b1b404660d8845250af3fb574eb21c69c74d8
Dependencies: #20130 Stopgaps:

Description (last modified by jdemeyer)

Replace dependencies like $(INST)/$(PARI) by pari.

configure tarball (for buildbot testing): ​http://sage.ugent.be/www/jdemeyer/sage/configure-147.tar.gz

Change History (30)

comment:1 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/dependencies__use__foo__instead_of____inst____foo__

comment:2 Changed 4 years ago by git

  • Commit set to bbb688a391cb5e3c1478e38c8d697a9042f43a36

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​bbb688adependencies: use "foo" instead of "$(INST)/$(FOO)"

comment:3 Changed 4 years ago by git

  • Commit changed from bbb688a391cb5e3c1478e38c8d697a9042f43a36 to 9145390c3d1884730e7b00993648e0f984e641cb

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​9145390dependencies: use "foo" instead of "$(INST)/$(FOO)"

comment:4 Changed 4 years ago by git

  • Commit changed from 9145390c3d1884730e7b00993648e0f984e641cb to f26772f0d19c4f137dec8bd2feb3f059c07e1570

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​f26772fdependencies: use "foo" instead of "$(INST)/$(FOO)"

comment:5 Changed 4 years ago by jdemeyer

  • Cc SimonKing added
  • Description modified (diff)

comment:6 Changed 4 years ago by git

  • Commit changed from f26772f0d19c4f137dec8bd2feb3f059c07e1570 to 87d5afd009c09cc8d15e8822c1dc51925aad7dd5

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​87d5afddependencies: use "foo" instead of "$(INST)/$(FOO)"

comment:7 Changed 4 years ago by git

  • Commit changed from 87d5afd009c09cc8d15e8822c1dc51925aad7dd5 to 463d22a4eb4555e3180aa25446f678a79686845d

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​463d22adependencies: use "foo" instead of "$(INST)/$(FOO)"

comment:8 Changed 4 years ago by jdemeyer

  • Cc vbraun added
  • Description modified (diff)
  • Status changed from new to needs_review

comment:9 Changed 4 years ago by embray

Ah, I was just thinking about this. Actually I had a broader proposal that I'd expand on later of cobbling all package metadata into a single machine-consumable file that I would like to use to better generate info on packages. But just improving the format of the "dependencies" file is a good start.

comment:10 Changed 4 years ago by jdemeyer

  • Cc fbissey added

comment:11 Changed 4 years ago by fbissey

I don't usually dwell in that area. It made me read some documentation. It does look ok and is a bit simpler.

comment:12 follow-up: Changed 4 years ago by vbraun

Lgtm but can you also merge in #20129

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

Replying to vbraun:

Lgtm but can you also merge in #20129

Why? There is no conflict (openblas has no dependencies).

comment:14 Changed 4 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Right, forgot about that.

comment:15 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

Conflicts with #20130

comment:16 Changed 4 years ago by git

  • Commit changed from 463d22a4eb4555e3180aa25446f678a79686845d to d7e720f74624b5690a57d8c7eee6da3bf8557cef

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

​18d59bfMove third-party packages to pkg-config blas
​de7efd2Fix typo: use lapack.pc not blas.pc
​f1302b2Bump patchlevels
​555b258Iml version/patchlevel fix
​d7e720fMerge commit '555b25811afe8da5d83c3ed6461e423a70f961f7' into t/20140/dependencies__use__foo__instead_of____inst____foo__

comment:17 Changed 4 years ago by jdemeyer

  • Dependencies set to #20130
  • Status changed from needs_work to positive_review

comment:18 Changed 4 years ago by git

  • Commit changed from d7e720f74624b5690a57d8c7eee6da3bf8557cef to 2e1cbc5ff6016d5d3ea1f2f79f770bc7c096365a
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​2e1cbc5Use sed to transform dependencies from "foo" to "$(inst_foo)"

comment:19 Changed 4 years ago by jdemeyer

Previous version didn't work because of phony targets in the dependencies. This new version still allows dependencies to be listed as foo. A small sed script transforms this to $(inst_foo).

comment:20 Changed 4 years ago by git

  • Commit changed from 2e1cbc5ff6016d5d3ea1f2f79f770bc7c096365a to 383b1b404660d8845250af3fb574eb21c69c74d8

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​383b1b4Use sed to transform dependencies from "foo" to "$(inst_foo)"

comment:21 follow-up: Changed 4 years ago by embray

While you're at it, I've noticed that some packages don't have all their dependencies listed. I don't have an exhaustive list yet, but I just encountered that jsonschema is only included as a dependency of nbformat even though it's not listed as such.

Another one: mistune is a dependency of nbconvert.

Last edited 4 years ago by embray (previous) (diff)

comment:22 Changed 4 years ago by vbraun

If we had a way of getting the dependency information into Python then it would be easy enough to compare with the pip dependency information for testing....

comment:23 in reply to: ↑ 21 Changed 4 years ago by jdemeyer

Replying to embray:

jsonschema is only included as a dependency of nbformat even though it's not listed as such.

I don't understand this sentence

comment:24 Changed 4 years ago by embray

The only reason jsonschema has an spkg is sage is because it's a dependency of nbformat, but nbformat does not list jsonschema in its dependencies.

comment:25 Changed 4 years ago by jdemeyer

Maybe you are confusing run-time dependencies with build-time dependencies? The dependencies files are really only meant for build-time dependencies.

Anyway, this isn't what this ticket is about...

comment:26 follow-up: Changed 4 years ago by embray

Okay, that must be it then. I'm confused because some of the Python packages do list dependencies that I wouldn't think of as build dependencies (unless maybe the package doesn't import without them).

comment:27 in reply to: ↑ 26 Changed 4 years ago by jdemeyer

Replying to embray:

I'm confused because some of the Python packages do list dependencies that I wouldn't think of as build dependencies (unless maybe the package doesn't import without them).

First of all, keep in mind that a test-suite is also considered "build-time". But it could very well be that packages list dependencies which are not strictly required. Usually, the time to build Python packages is negligible, so it's not so important.

comment:28 Changed 4 years ago by jdemeyer

Regardless of all this, can somebody review this ticket please?

Last edited 4 years ago by jdemeyer (previous) (diff)

comment:29 Changed 4 years ago by vbraun

  • Status changed from needs_review to positive_review

comment:30 Changed 4 years ago by vbraun

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