Re: [PATCH 11/11] Allow ETC_GITCONFIG to be a relative path.

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

 




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
-
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]

  Powered by Linux