From: Steffen Prohaska <prohaska@zib.de>
Date: Thu, 15 Nov 2007 22:24:52 GMT
Subject: Re: [PATCH 11/11] Allow ETC_GITCONFIG to be a relative path.
Message-ID: <1046B46D-585D-47B6-91B3-38F760E68DD4@zib.de>
In-Reply-To: <200711152243.10229.johannes.sixt@telecom.at>


On Nov 15, 2007, at 10:43 PM, Johannes Sixt wrote:

> On Thursday 15 November 2007 21:10, Steffen Prohaska wrote:
>> On Nov 15, 2007, at 7:31 PM, Johannes Sixt wrote:
>>> On Thursday 15 November 2007 07:53, Steffen Prohaska wrote:
>>>> Now I'm wondering if we could make path relocation a bit more
>>>> explicit.  How about doing something like.
>>>>
>>>> 	system_wide = relocate_path(ETC_GITCONFIG);
>>>>
>>>> and relocate_path(const char *) would expand a format
>>>> string in path.  At this point I see only a single %s
>>>> that would be expanded with the install prefix.  If
>>>> ETC_GITCONFIG is "%s/etc/gitconfig" relocate path will return
>>>> "C:/msysgit/bin/etc/gitconfig" for my above example.  It is
>>>> basically a printf with the install prefix path.
>>>
>>> I don't see the problem that you are trying to solve.
>>
>> Path relocation would be more explicit:
>> 1) Paths that need to be relocated are filtered through
>>     relocate_path().  That's easy to understand.
>> 2) All the code prefixing the path is in relocate_path().
>>     This avoids code duplication.
>> 3) Path that should be relocated contain "%s" in the Makefile.
>>     This indicates that some string expansion may take place.
>>
>> (1) and (2) would be useful even if you do not agree with (3).
>> The code in PATCH 9/11 and PATCH 11/11 looks very similar.
>> If the prefixing code went into a separate function, we'd
>> have less code.  Also, relocate_path() could be useful at
>> other places.  For example, I'd use it to locate the HTML
>> documentation relative to the installation directory.
>
> How is %s/foo different from ../foo? There are only 2 paths that  
> need to be
> relocatable. Your proposal is over-engineering, IMHO.

fair enough. Let's take your patches.

	Steffen