Opened 6 years ago
Closed 6 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, GitHub, GitLab) | Commit: | 383b1b404660d8845250af3fb574eb21c69c74d8 |
Dependencies: | #20130 | Stopgaps: |
Description (last modified by )
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 6 years ago by
- Branch set to u/jdemeyer/dependencies__use__foo__instead_of____inst____foo__
comment:2 Changed 6 years ago by
- Commit set to bbb688a391cb5e3c1478e38c8d697a9042f43a36
comment:3 Changed 6 years ago by
- Commit changed from bbb688a391cb5e3c1478e38c8d697a9042f43a36 to 9145390c3d1884730e7b00993648e0f984e641cb
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9145390 | dependencies: use "foo" instead of "$(INST)/$(FOO)"
|
comment:4 Changed 6 years ago by
- Commit changed from 9145390c3d1884730e7b00993648e0f984e641cb to f26772f0d19c4f137dec8bd2feb3f059c07e1570
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f26772f | dependencies: use "foo" instead of "$(INST)/$(FOO)"
|
comment:5 Changed 6 years ago by
- Cc SimonKing added
- Description modified (diff)
comment:6 Changed 6 years ago by
- Commit changed from f26772f0d19c4f137dec8bd2feb3f059c07e1570 to 87d5afd009c09cc8d15e8822c1dc51925aad7dd5
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
87d5afd | dependencies: use "foo" instead of "$(INST)/$(FOO)"
|
comment:7 Changed 6 years ago by
- Commit changed from 87d5afd009c09cc8d15e8822c1dc51925aad7dd5 to 463d22a4eb4555e3180aa25446f678a79686845d
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
463d22a | dependencies: use "foo" instead of "$(INST)/$(FOO)"
|
comment:8 Changed 6 years ago by
- Cc vbraun added
- Description modified (diff)
- Status changed from new to needs_review
comment:9 Changed 6 years ago by
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 6 years ago by
- Cc fbissey added
comment:11 Changed 6 years ago by
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: ↓ 13 Changed 6 years ago by
Lgtm but can you also merge in #20129
comment:13 in reply to: ↑ 12 Changed 6 years ago by
comment:14 Changed 6 years ago by
- Reviewers set to Volker Braun
- Status changed from needs_review to positive_review
Right, forgot about that.
comment:15 Changed 6 years ago by
- Status changed from positive_review to needs_work
Conflicts with #20130
comment:16 Changed 6 years ago by
- Commit changed from 463d22a4eb4555e3180aa25446f678a79686845d to d7e720f74624b5690a57d8c7eee6da3bf8557cef
Branch pushed to git repo; I updated commit sha1. New commits:
18d59bf | Move third-party packages to pkg-config blas
|
de7efd2 | Fix typo: use lapack.pc not blas.pc
|
f1302b2 | Bump patchlevels
|
555b258 | Iml version/patchlevel fix
|
d7e720f | Merge commit '555b25811afe8da5d83c3ed6461e423a70f961f7' into t/20140/dependencies__use__foo__instead_of____inst____foo__
|
comment:17 Changed 6 years ago by
- Dependencies set to #20130
- Status changed from needs_work to positive_review
comment:18 Changed 6 years ago by
- 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:
2e1cbc5 | Use sed to transform dependencies from "foo" to "$(inst_foo)"
|
comment:19 Changed 6 years ago by
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 6 years ago by
- Commit changed from 2e1cbc5ff6016d5d3ea1f2f79f770bc7c096365a to 383b1b404660d8845250af3fb574eb21c69c74d8
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
383b1b4 | Use sed to transform dependencies from "foo" to "$(inst_foo)"
|
comment:21 follow-up: ↓ 23 Changed 6 years ago by
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
.
comment:22 Changed 6 years ago by
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 6 years ago by
Replying to embray:
jsonschema
is only included as a dependency ofnbformat
even though it's not listed as such.
I don't understand this sentence
comment:24 Changed 6 years ago by
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 6 years ago by
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: ↓ 27 Changed 6 years ago by
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 6 years ago by
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 6 years ago by
Regardless of all this, can somebody review this ticket please?
comment:29 Changed 6 years ago by
- Status changed from needs_review to positive_review
comment:30 Changed 6 years ago by
- 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
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
dependencies: use "foo" instead of "$(INST)/$(FOO)"