bpo-31934: Abort when building out of a not clean source tree by xdegaye · Pull Request #4255 · python/cpython (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation17 Commits4 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I was bitten multiple times by this annoying bug. It may useful to backport this helper to Python 2.7 and 3.6.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add this dependency to $(BUILD_PYTHON) instead, to fix more "make" commands?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is much better and would cover all cases I think.

BTW a '.PHONY' declaration is missing for check-clean-src.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually Programs/python.o is the first object built by the Makefile as it is the first prerequisite in the $(BUILDPYTHON) rule.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :-)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to put check-clean-src in $(BUILDPYTHON). I tested, it works:

# Build the interpreter <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mo stretchy="false">(</mo><mi>B</mi><mi>U</mi><mi>I</mi><mi>L</mi><mi>D</mi><mi>P</mi><mi>Y</mi><mi>T</mi><mi>H</mi><mi>O</mi><mi>N</mi><mo stretchy="false">)</mo><mo>:</mo><mi>c</mi><mi>h</mi><mi>e</mi><mi>c</mi><mi>k</mi><mo>−</mo><mi>c</mi><mi>l</mi><mi>e</mi><mi>a</mi><mi>n</mi><mo>−</mo><mi>s</mi><mi>r</mi><mi>c</mi><mi>P</mi><mi>r</mi><mi>o</mi><mi>g</mi><mi>r</mi><mi>a</mi><mi>m</mi><mi>s</mi><mi mathvariant="normal">/</mi><mi>p</mi><mi>y</mi><mi>t</mi><mi>h</mi><mi>o</mi><mi>n</mi><mi mathvariant="normal">.</mi><mi>o</mi></mrow><annotation encoding="application/x-tex">(BUILDPYTHON):	check-clean-src Programs/python.o </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mopen">(</span><span class="mord mathnormal" style="margin-right:0.05017em;">B</span><span class="mord mathnormal" style="margin-right:0.10903em;">U</span><span class="mord mathnormal" style="margin-right:0.07847em;">I</span><span class="mord mathnormal">L</span><span class="mord mathnormal" style="margin-right:0.02778em;">D</span><span class="mord mathnormal" style="margin-right:0.13889em;">P</span><span class="mord mathnormal" style="margin-right:0.22222em;">Y</span><span class="mord mathnormal" style="margin-right:0.13889em;">T</span><span class="mord mathnormal" style="margin-right:0.08125em;">H</span><span class="mord mathnormal" style="margin-right:0.10903em;">ON</span><span class="mclose">)</span><span class="mspace" style="margin-right:0.2778em;"></span><span class="mrel">:</span><span class="mspace" style="margin-right:0.2778em;"></span></span><span class="base"><span class="strut" style="height:0.7778em;vertical-align:-0.0833em;"></span><span class="mord mathnormal">c</span><span class="mord mathnormal">h</span><span class="mord mathnormal">ec</span><span class="mord mathnormal" style="margin-right:0.03148em;">k</span><span class="mspace" style="margin-right:0.2222em;"></span><span class="mbin">−</span><span class="mspace" style="margin-right:0.2222em;"></span></span><span class="base"><span class="strut" style="height:0.7778em;vertical-align:-0.0833em;"></span><span class="mord mathnormal">c</span><span class="mord mathnormal" style="margin-right:0.01968em;">l</span><span class="mord mathnormal">e</span><span class="mord mathnormal">an</span><span class="mspace" style="margin-right:0.2222em;"></span><span class="mbin">−</span><span class="mspace" style="margin-right:0.2222em;"></span></span><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mord mathnormal">src</span><span class="mord mathnormal" style="margin-right:0.13889em;">P</span><span class="mord mathnormal">ro</span><span class="mord mathnormal" style="margin-right:0.03588em;">g</span><span class="mord mathnormal" style="margin-right:0.02778em;">r</span><span class="mord mathnormal">am</span><span class="mord mathnormal">s</span><span class="mord">/</span><span class="mord mathnormal">p</span><span class="mord mathnormal" style="margin-right:0.03588em;">y</span><span class="mord mathnormal">t</span><span class="mord mathnormal">h</span><span class="mord mathnormal">o</span><span class="mord mathnormal">n</span><span class="mord">.</span><span class="mord mathnormal">o</span></span></span></span>(LIBRARY) <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mo stretchy="false">(</mo><mi>L</mi><mi>D</mi><mi>L</mi><mi>I</mi><mi>B</mi><mi>R</mi><mi>A</mi><mi>R</mi><mi>Y</mi><mo stretchy="false">)</mo></mrow><annotation encoding="application/x-tex">(LDLIBRARY) </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mopen">(</span><span class="mord mathnormal">L</span><span class="mord mathnormal" style="margin-right:0.02778em;">D</span><span class="mord mathnormal">L</span><span class="mord mathnormal" style="margin-right:0.07847em;">I</span><span class="mord mathnormal" style="margin-right:0.00773em;">BR</span><span class="mord mathnormal">A</span><span class="mord mathnormal" style="margin-right:0.00773em;">R</span><span class="mord mathnormal" style="margin-right:0.22222em;">Y</span><span class="mclose">)</span></span></span></span>(PY3LIBRARY)
 <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mo stretchy="false">(</mo><mi>L</mi><mi>I</mi><mi>N</mi><mi>K</mi><mi>C</mi><mi>C</mi><mo stretchy="false">)</mo></mrow><annotation encoding="application/x-tex">(LINKCC) </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mopen">(</span><span class="mord mathnormal">L</span><span class="mord mathnormal" style="margin-right:0.07847em;">I</span><span class="mord mathnormal" style="margin-right:0.10903em;">N</span><span class="mord mathnormal" style="margin-right:0.07153em;">K</span><span class="mord mathnormal" style="margin-right:0.07153em;">CC</span><span class="mclose">)</span></span></span></span>(PY_LDFLAGS) <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mo stretchy="false">(</mo><mi>L</mi><mi>I</mi><mi>N</mi><mi>K</mi><mi>F</mi><mi>O</mi><mi>R</mi><mi>S</mi><mi>H</mi><mi>A</mi><mi>R</mi><mi>E</mi><mi>D</mi><mo stretchy="false">)</mo><mo>−</mo><mi>o</mi></mrow><annotation encoding="application/x-tex">(LINKFORSHARED) -o </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mopen">(</span><span class="mord mathnormal">L</span><span class="mord mathnormal" style="margin-right:0.07847em;">I</span><span class="mord mathnormal" style="margin-right:0.10903em;">N</span><span class="mord mathnormal" style="margin-right:0.07153em;">K</span><span class="mord mathnormal" style="margin-right:0.05764em;">FORS</span><span class="mord mathnormal" style="margin-right:0.08125em;">H</span><span class="mord mathnormal">A</span><span class="mord mathnormal" style="margin-right:0.05764em;">RE</span><span class="mord mathnormal" style="margin-right:0.02778em;">D</span><span class="mclose">)</span><span class="mspace" style="margin-right:0.2222em;"></span><span class="mbin">−</span><span class="mspace" style="margin-right:0.2222em;"></span></span><span class="base"><span class="strut" style="height:0.4306em;"></span><span class="mord mathnormal">o</span></span></span></span>@ Programs/python.o <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mo stretchy="false">(</mo><mi>B</mi><mi>L</mi><mi>D</mi><mi>L</mi><mi>I</mi><mi>B</mi><mi>R</mi><mi>A</mi><mi>R</mi><mi>Y</mi><mo stretchy="false">)</mo></mrow><annotation encoding="application/x-tex">(BLDLIBRARY) </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mopen">(</span><span class="mord mathnormal" style="margin-right:0.05017em;">B</span><span class="mord mathnormal">L</span><span class="mord mathnormal" style="margin-right:0.02778em;">D</span><span class="mord mathnormal">L</span><span class="mord mathnormal" style="margin-right:0.07847em;">I</span><span class="mord mathnormal" style="margin-right:0.00773em;">BR</span><span class="mord mathnormal">A</span><span class="mord mathnormal" style="margin-right:0.00773em;">R</span><span class="mord mathnormal" style="margin-right:0.22222em;">Y</span><span class="mclose">)</span></span></span></span>(LIBS) <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mo stretchy="false">(</mo><mi>M</mi><mi>O</mi><mi>D</mi><mi>L</mi><mi>I</mi><mi>B</mi><mi>S</mi><mo stretchy="false">)</mo></mrow><annotation encoding="application/x-tex">(MODLIBS) </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mopen">(</span><span class="mord mathnormal" style="margin-right:0.02778em;">MO</span><span class="mord mathnormal" style="margin-right:0.02778em;">D</span><span class="mord mathnormal">L</span><span class="mord mathnormal" style="margin-right:0.07847em;">I</span><span class="mord mathnormal" style="margin-right:0.05764em;">BS</span><span class="mclose">)</span></span></span></span>(SYSLIBS) $(LDLAST)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1. 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.

1. Another minor problem is that the recipe of check-clean-src may be run multiple times.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh oh, I didn't know.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ;-)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh oh, I didn't know.

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 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 0de92859caf25e65fc968d4bb68626e9ba21b851 3.6

Sorry, @xdegaye, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 0de92859caf25e65fc968d4bb68626e9ba21b851 2.7

xdegaye added a commit that referenced this pull request

Nov 8, 2017

xdegaye added a commit that referenced this pull request

Nov 8, 2017

Labels

type-bug

An unexpected behavior, bug, or error