#22094 closed defect (fixed)
setup.py: run_autogen is ran too late
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | sage-7.5 |
Component: | build | Keywords: | |
Cc: | egourgoulhon, embray, fbissey | Merged in: | |
Authors: | François Bissey | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | 8e172f6 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
When building Sage from scratch, the following happens:
ImportError: No module named interpreters.wrapper_rdf
In src/setup.py
, the method run_autogen
is ran after discovering and building the Python modules. So, the Python module sage/ext/interpreters/__init__.py
is not installed.
This problem was introduced in #21613.
Change History (22)
comment:1 Changed 6 years ago by
- Cc fbissey added
comment:2 Changed 6 years ago by
comment:3 Changed 6 years ago by
OK I can reproduce it at will here by taking a full build and then doing MAKE="make -j8" ./sage -ba
(not sure if it fails with a single threaded build). ./sage
will crash and complain about interpreters.wrapper_rdf
in the crash report.
comment:4 follow-up: ↓ 6 Changed 6 years ago by
I just reproduced the problem by building Sage from scratch (make distclean; make
). I thought the buildbots would test that? In the process, I found another problem: #22098.
comment:5 Changed 6 years ago by
- Description modified (diff)
- Summary changed from ImportError: No module named interpreters.wrapper_rdf to setup.py: run_autogen is ran too late
comment:6 in reply to: ↑ 4 ; follow-up: ↓ 7 Changed 6 years ago by
Here are some experiments regarding the reproductibility on my system (Ubuntu 16.04):
- the error seems systematic: i.e. running
MAKE="make -j8" make
in a new fresh git clone yields the same error; then runningmake
finishes the build without any error - to determine whether this due to the parallel build, I ran simply
make
in another fresh git clone: then the build crashes due to another error: thepsutil
one reported in #22098
comment:7 in reply to: ↑ 6 ; follow-up: ↓ 8 Changed 6 years ago by
With the correction proposed in #22098, the non-parallelized build with make
goes further but
halts at the same error as reported in the current ticket, i.e.
[dochtml] File "sage/ext/interpreters/wrapper_rdf.pxd", line 7, in init sage.plot.plot3d.parametric_surface (/home/eric/sage/7.5.rc0-22098/src/build/cythonized/sage/plot/plot3d/parametric_surface.c:12253) [dochtml] ImportError: No module named interpreters.wrapper_rdf
So clearly the issue is not due to the parallel build.
comment:8 in reply to: ↑ 7 Changed 6 years ago by
Replying to egourgoulhon:
So clearly the issue is not due to the parallel build.
As for the parallel build, running make
a second time leads to a successful build.
In other words, make && make
works, while a single make
fails...
comment:9 Changed 6 years ago by
The buildbot tests building from scratch but for some reasons succeeds... there is most likely a random component to it.
comment:10 Changed 6 years ago by
On the machine it fails, it looks quite systematic. Something in the environment?
comment:11 Changed 6 years ago by
I think it's pointless to figure out the exact circumstances when the problem occurs. I analysed it well enough (see ticket description) that somebody could fix it.
comment:12 Changed 6 years ago by
- Description modified (diff)
comment:13 Changed 6 years ago by
- Branch set to u/fbissey/wrapper
- Commit set to 6c2a4caaaeea3b6082936d57c48e8a949ebe3e00
- Status changed from new to needs_review
OK, this branch works for me but I don't know if it is the most appropriate way to do it. Needs review.
New commits:
6c2a4ca | Move autogen to a more appropriate location.
|
comment:14 follow-up: ↓ 15 Changed 6 years ago by
- Status changed from needs_review to needs_work
It's not "Generating Interpreter Sources", it's "Generating auto-generated sources". So both the comment and the print
statement are wrong in this regard.
About the actual solution: I'm pretty sure that Erik won't like it (in setup.py
, you are supposed to only do stuff within the setup()
call). However, this fixes a blocker issue, so I am OK with this branch being a stopgap.
comment:15 in reply to: ↑ 14 Changed 6 years ago by
Replying to jdemeyer:
It's not "Generating Interpreter Sources", it's "Generating auto-generated sources". So both the comment and the
Amusing considering I copied pasted the original print
statement from Erik. I can see it was more general before #21613.
About the actual solution: I'm pretty sure that Erik won't like it (in
setup.py
, you are supposed to only do stuff within thesetup()
call). However, this fixes a blocker issue, so I am OK with this branch being a stopgap.
OK for the stopgap status. If Erik, think it can be done better he is welcome to do it, but it is the last blocker for 7.5 so getting things out is important. New commit coming soon.
comment:16 Changed 6 years ago by
- Commit changed from 6c2a4caaaeea3b6082936d57c48e8a949ebe3e00 to 8e172f646e284ed98b3c0d57dd0a43792084efcc
Branch pushed to git repo; I updated commit sha1. New commits:
8e172f6 | amending coment and message in setup.py
|
comment:17 Changed 6 years ago by
- Status changed from needs_work to needs_review
I also forgotten, I wanted to include the ticket number in the comment, so that was a good opportunity.
comment:18 Changed 6 years ago by
- Reviewers set to Jeroen Demeyer
comment:19 Changed 6 years ago by
Just to tell that François' commit fixes the build from scratch for me too.
comment:20 Changed 6 years ago by
- Status changed from needs_review to positive_review
comment:21 Changed 6 years ago by
- Branch changed from u/fbissey/wrapper to 8e172f646e284ed98b3c0d57dd0a43792084efcc
- Resolution set to fixed
- Status changed from positive_review to closed
comment:22 Changed 6 years ago by
- Commit 8e172f646e284ed98b3c0d57dd0a43792084efcc deleted
Follow-up: #22106
Untested hypothesis: the "Discovering Python/Cython source code" part below is run before auto-generating the sources, which is the wrong order:
The auto-generated files might not be discovered this way, leading to those files being considered for cleaning.