Re: [PATCH 1/2] remote-helpers: tests: use python directly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 17, 2013 at 9:51 PM, David Aguilar <davvid@xxxxxxxxx> wrote:
> On Fri, May 17, 2013 at 7:12 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>>
>>> These remote helpers use 'env python', not PYTHON_PATH, so that's where
>>> we should check for the extensions. Otherwise, if 'python' is not
>>> PYTHON_PATH (e.g. /usr/bin/python: Makefile's default), there will be a
>>> mismatch between the python libraries actually accessible to the remote
>>> helpers.
>>
>> What I am reading here is that what the "helper" uses and what the
>> "test" checks to see if it can use the "helper" were different; and
>> this patch fixes that misalignment by testing what the "helper"
>> actually uses.
>>
>> So it is a right thing to do in that sense.
>>
>> I however am having this nagging feeling that I may be missing
>> something subtle here.  Comments from others are very much welcome.
>
> Yes, this is correct.  Another way to skin this cat would be to do
> search/replace in a Makefile to burn in the PYTHON_PATH similar to how
> we do for the .sh scripts and other .py files in the main Makefile.
> The remote helpers are in contrib/ so they do not go through the main
> Makefile, which is the root cause.
>
> Longer-term, it would be good to treat these uniformly, but this is no
> worse for now.

Yeap, I initially thought it would be tricky to implement in the
rather minimal Makefile, but it seems it wouldn't. Whoever, I still
find it useful that I don't have to run 'make' to test these, and I
think it's nice that people can download directly as the final name
('git-remote-hg'), and don't have to rename. And since these aren't
installed with 'make install' anyway, I don't see any big hurry.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]