Opened 7 years ago
Closed 7 years ago
#18842 closed defect (fixed)
Really fix cleaning of Sage library
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.8 |
Component: | cython | Keywords: | |
Cc: | fbissey, ncohen | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Steven Trogdon |
Report Upstream: | N/A | Work issues: | |
Branch: | 2b0fbaf (Commits, GitHub, GitLab) | Commit: | 2b0fbaff82a95a4345b8aa70d4af6965a1a23126 |
Dependencies: | Stopgaps: |
Description (last modified by )
Since #18494, we install .pxd
files but we never remove them.
Change History (15)
comment:1 Changed 7 years ago by
- Cc ncohen added
comment:2 Changed 7 years ago by
comment:3 Changed 7 years ago by
Actually, only fast_digraph.pxd
and vertex_separation.pxd
have to be removed which are new with ticket #18746.
comment:4 Changed 7 years ago by
- Branch set to u/jdemeyer/cythonization_broken
comment:5 Changed 7 years ago by
- Commit set to 2b0fbaff82a95a4345b8aa70d4af6965a1a23126
- Summary changed from Cythonization broken to Really fix cleaning of Sage library
New commits:
2b0fbaf | Consider all installed files for cleaning up
|
comment:6 follow-up: ↓ 8 Changed 7 years ago by
So i see you are switching to only using data_files
and the only headers to be considered are those found in SAGE_CYTHONIZED
which makes sense as all the useful headers will be copied.
Is the issue only happening when one does an upgrade or a re-build where as your commit suggest we may have obsolete .pxd
, .pyx
or .h
files being kept while they should have been deleted?
comment:7 follow-up: ↓ 10 Changed 7 years ago by
Hmm... the basic idea of my commit works, but the problem is the order: this cleaning needs to be done before cythonization.
comment:8 in reply to: ↑ 6 Changed 7 years ago by
Replying to fbissey:
Is the issue only happening when one does an upgrade or a re-build where as your commit suggest we may have obsolete
.pxd
,.pyx
or.h
files being kept while they should have been deleted?
Yes, if we delete a .pxd
file from the sources, it should also be deleted from the installation directory.
comment:9 Changed 7 years ago by
- Description modified (diff)
- Report Upstream changed from N/A to Reported upstream. No feedback yet.
comment:10 in reply to: ↑ 7 Changed 7 years ago by
Replying to jdemeyer:
Hmm... the basic idea of my commit works, but the problem is the order: this cleaning needs to be done before cythonization.
Bummer. You're absolutely correct. And this is what is done when they are removed manually.
comment:11 Changed 7 years ago by
- Report Upstream changed from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.
comment:12 Changed 7 years ago by
- Description modified (diff)
- Priority changed from blocker to major
- Report Upstream changed from Reported upstream. Developers acknowledge bug. to N/A
- Status changed from new to needs_review
See #18851 for the Cython patch.
This branch is still relevant, but it doesn't fix the problem originally reported.
comment:13 Changed 7 years ago by
- Reviewers set to Steven Trogdon
- Status changed from needs_review to positive_review
comment:14 Changed 7 years ago by
It's absolutely fine with me and definitely should go in.
comment:15 Changed 7 years ago by
- Branch changed from u/jdemeyer/cythonization_broken to 2b0fbaff82a95a4345b8aa70d4af6965a1a23126
- Resolution set to fixed
- Status changed from positive_review to closed
Here a
seemed to allow one to recover, but then required a
make doc-clean && make
to rebuild the docs.