Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(86598)

#22359: Remove incorrect uses of recursive make

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 2 months ago by jonas.wagner
Modified:
3 years, 6 months ago
Reviewers:
vadmium+py
CC:
loewis, brett.cannon, doko, AntoinePitrou, Arfrever, xdegaye, devnull_psf.upfronthosting.co.za, Martin Panter, koobs, jonas.wagner_epfl.ch, WanderingLogic
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Total comments: 2

Patch Set 5 #

Patch Set 6 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
configure View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
configure.ac View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
Makefile.pre.in View 1 2 3 4 5 3 chunks +20 lines, -7 lines 0 comments Download

Messages

Total messages: 1
Martin Panter
3 years, 6 months ago #1
http://bugs.python.org/review/22359/diff/17000/Makefile.pre.in
File Makefile.pre.in (right):

http://bugs.python.org/review/22359/diff/17000/Makefile.pre.in#newcode272
Makefile.pre.in:272: GRAMMAR_H=	$(srcdir)/Include/graminit.h
I understand these are placed in the output directory on purpose. It would be
good to avoid messing with the source files in a normal build (especially if
they are read-only). This could do with a comment though. If you look at the
history, this has switched back and forth too many times:

https://hg.python.org/cpython/rev/740290a56190 (support VPATH)
https://hg.python.org/cpython/rev/c9f23f80fc93 (generate in build dir)
https://hg.python.org/cpython/rev/67ed8a6905c3 (generate in srcdir but ignore
error)
https://hg.python.org/cpython/rev/2a773bef40c2 (recognize error)
https://hg.python.org/cpython/rev/62044cd5b19b (generate in build dir)

I guess you changed this because the rules do not create these files any more in
cross-compile mode. Non-phony rules should generate their target files, so in
this case I think you should copy or link the files into the build directory if
they are not already there.

http://bugs.python.org/review/22359/diff/17000/Makefile.pre.in#newcode726
Makefile.pre.in:726: $(srcdir)/Lib/importlib/_bootstrap_external.py
Python/importlib_external.h; \
There is a second tab as indentation here. I wonder if it would be better to
change it to spaces (the line above uses spaces)?
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+