bpo-31934: Abort when building out of a not clean source tree #4255
Conversation
xdegaye
added
needs backport to 2.7
type-bug
Makefile.pre.in
Outdated
| @@ -439,9 +439,16 @@ DTRACE_DEPS = \ | |||
| # Rules | |||
|
|
|||
| # Default target | |||
| all: @DEF_MAKE_ALL_RULE@ | |||
| all: check-clean-src @DEF_MAKE_ALL_RULE@ | |||
There was a problem hiding this comment.
Would it be possible to add this dependency to $(BUILD_PYTHON) instead, to fix more "make" commands?
There was a problem hiding this comment.
Yes, this is much better and would cover all cases I think.
BTW a '.PHONY' declaration is missing for check-clean-src.
Makefile.pre.in
Outdated
| build_all: $(BUILDPYTHON) oldsharedmods sharedmods gdbhooks Programs/_testembed python-config | ||
|
|
||
| # Check that the source is clean when building out of source. | ||
| check-clean-src: | ||
| @if test -n "$(VPATH)" -a -f "$(srcdir)/Programs/python.o"; then \ |
There was a problem hiding this comment.
I suggest to use Objects/object.o instead, since the compilation may have succeed to build object.o but failed to build python.o (or failed before trying to build python.o).
There was a problem hiding this comment.
Actually Programs/python.o is the first object built by the Makefile as it is the first prerequisite in the $(BUILDPYTHON) rule.
There was a problem hiding this comment.
Actually Programs/python.o is the first object built by the Makefile as it is the first prerequisite in the $(BUILDPYTHON) rule.
Oh ok, I see. I that case, it's fine :-)
| @@ -440,7 +440,15 @@ DTRACE_DEPS = \ | |||
|
|
|||
| # Default target | |||
| all: @DEF_MAKE_ALL_RULE@ | |||
| build_all: $(BUILDPYTHON) oldsharedmods sharedmods gdbhooks Programs/_testembed python-config | |||
| build_all: check-clean-src $(BUILDPYTHON) oldsharedmods sharedmods gdbhooks \ | |||
There was a problem hiding this comment.
I would prefer to put check-clean-src in $(BUILDPYTHON). I tested, it works:
# Build the interpreter
$(BUILDPYTHON): check-clean-src Programs/python.o $(LIBRARY) $(LDLIBRARY) $(PY3LIBRARY)
$(LINKCC) $(PY_LDFLAGS) $(LINKFORSHARED) -o $@ Programs/python.o $(BLDLIBRARY) $(LIBS) $(MODLIBS) $(SYSLIBS) $(LDLAST)
There was a problem hiding this comment.
- One problem is that in this case the $(BUILDPYTHON) target is rebuilt even when it is up to date, for the following reason:
A phony target should not be a prerequisite of a real target file;
if it is, its recipe will be run every time make goes to update that file.
This is a quote from Phony Targets.
- Another minor problem is that the recipe of check-clean-src may be run multiple times.
Makefile.pre.in
Outdated
| # Check that the source is clean when building out of source. | ||
| check-clean-src: | ||
| @if test -n "$(VPATH)" -a -f "$(srcdir)/Programs/python.o"; then \ | ||
| echo "Error: The source directory is not clean" ; \ |
There was a problem hiding this comment.
The error message is not easy to understand. I propose:
echo "Error: The source directory ($(srcdir)) is not clean" ; \
echo "Building Python out of the source tree ($(abs_builddir)) requires a clean source tree ($(abs_srcdir)" ; \
echo "Try to run: make -C \"$(srcdir)\" clean" ; \
At least, the proposed command worked for me ;-)
| @@ -440,7 +440,15 @@ DTRACE_DEPS = \ | |||
|
|
|||
| # Default target | |||
| all: @DEF_MAKE_ALL_RULE@ | |||
| build_all: $(BUILDPYTHON) oldsharedmods sharedmods gdbhooks Programs/_testembed python-config | |||
| build_all: check-clean-src $(BUILDPYTHON) oldsharedmods sharedmods gdbhooks \ | |||
|
Thanks @xdegaye for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6. |
|
Sorry, @xdegaye, I could not cleanly backport this to |
|
Sorry, @xdegaye, I could not cleanly backport this to |
|
GH-4340 is a backport of this pull request to the 3.6 branch. |
|
GH-4342 is a backport of this pull request to the 2.7 branch. |
https://bugs.python.org/issue31934