Re: [PATCH 3/8] git_remote_helpers: Force rebuild if python version changes
- From
John Keeping <john@keeping.me.uk>
- Date
- Jan 13, 2013, 17:52 UTC
- Message-ID
- <20130113175238.GO4574@serenity.lan>
- In-Reply-To
- <20130113171402.GA1307@padd.com>
On Sun, Jan 13, 2013 at 12:14:02PM -0500, Pete Wyckoff wrote:
Show 36 quoted lines
> john@keeping.me.uk wrote on Sun, 13 Jan 2013 16:26 +0000:
>> On Sat, Jan 12, 2013 at 06:30:44PM -0500, Pete Wyckoff wrote:
>> > john@keeping.me.uk wrote on Sat, 12 Jan 2013 19:23 +0000:
>> >> When different version of python are used to build via distutils, the
>> >> behaviour can change. Detect changes in version and pass --force in
>> >> this case.
>> >[..]
>> >> diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
>> >[..]
>> >> +py_version=$(shell $(PYTHON_PATH) -c \
>> >> + 'import sys; print("%i.%i" % sys.version_info[:2])')
>> >> +
>> >> all: $(pysetupfile)
>> >> - $(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
>> >> + $(QUIET)test "$$(cat GIT-PYTHON_VERSION 2>/dev/null)" = "$(py_version)" || \
>> >> + flags=--force; \
>> >> + $(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build $$flags
>> >> + $(QUIET)echo "$(py_version)" >GIT-PYTHON_VERSION
>> >
>> > Can you depend on ../GIT-PYTHON-VARS instead? It comes from
>> > 96a4647 (Makefile: detect when PYTHON_PATH changes, 2012-12-18).
>> > It doesn't check version, just path, but hopefully that's good
>> > enough. I'm imagining a rule that would do "clean" if
>> > ../GIT-PYTHON-VARS changed, then build without --force.
>>
>> I was trying to keep the git_remote_helpers directory self contained. I
>> can't see how to depend on ../GIT-PYTHON-VARS in a way that is as simple
>> as this and keeps "make -C git_remote_helpers" working in a clean tree.
>>
>> Am I missing something obvious here?
>
> Not if it wants to stay self-contained; you're right.
>
> I'm not thrilled with how git_remote_helpers/Makefile always
> runs setup.py, and always generates PYLIBDIR, and now always
> invokes python a third time to see if its version changed.I don't think PYLIBDIR will be calculated unless it's used ('=' not ':=' means its a deferred variable).
I wonder if the version check should move into setup.py - it would be just as easy to check the file there and massage sys.args, although possibly not as neat.
John