Re: Mac OS 9 (Lamp) port
On May 30, 2010, at 8:19 PM, Jonathan Nieder wrote:
> Joshua Juran wrote:
>
>> http://github.com/jjuran/git.git
Apparently <http://github.com/jjuran/git> works better.
> I assume the lamp-git branch is the interesting one. Some of
The lamp-git branch is the one you'd checkout to build git. It merges the interesting ones.
> these patches (e.g., a3b378781) look like they might be of general
$ git show a3b378781
commit a3b378781228d59a13ee055d06c8f5465b543178
Author: Joshua Juran <jjuran@metamage.com>
Date: Sun Nov 29 03:57:54 2009 -0800
Avoid pointless use of one-celled array (which also confounds
Metrowerks C).
Pass the address of fetch_refspec_str directly to
parse_refspec_internal()
instead of copying fetch_refspec_str to an array and passing that. Metrowerks C won't initialize aggregates with values determined
at runtime, and
we can't use the C++ translator for parse_refspec_internal()
because it uses the
same identifier (refspec) for a variable as an existing struct
type. That, and there's no purpose to the array in the first place.
Show changes to 2 files +3 −4
remote.c, server-info.c
diff --git a/remote.c b/remote.c
index c70181c..ade0424 100644
--- a/remote.c
+++ b/remote.c
@@ -657,10 +657,9 @@ static struct refspec *parse_refspec_internal
(int nr_refspec, const char **refsp
int valid_fetch_refspec(const char *fetch_refspec_str)
{
- const char *fetch_refspec[] = { fetch_refspec_str };
struct refspec *refspec;
- refspec = parse_refspec_internal(1, fetch_refspec, 1, 1);
+ refspec = parse_refspec_internal(1, &fetch_refspec_str, 1, 1);
free_refspecs(refspec, 1);
return !!refspec;
}
Yes, this is the patch I suggested to Gary V. Vaughan.
> interest, while others (e.g., 6e6b106d) reveal Metrowerks to be
> a bit braindead.
$ git show 6e6b106d
commit 6e6b106dd6c561db726f90e2db07beeef1f716f9
Author: Joshua Juran <jjuran@metamage.com>
Date: Wed Apr 8 01:51:04 2009 -0700
Cast compare_info() arguments to (const struct pack_info *const *).
Otherwise, Metrowerks C reports "illegal implicit conversion
from 'const void *'
to 'struct pack_info *const *'".
diff --git a/server-info.c b/server-info.c
index 4098ca2..a9a0188 100644
--- a/server-info.c
+++ b/server-info.c
@@ -132,8 +132,8 @@ static int read_pack_info_file(const char *infofile)
static int compare_info(const void *a_, const void *b_)
{
- struct pack_info *const *a = a_;
- struct pack_info *const *b = b_;
+ const struct pack_info *const *a = a_;
+ const struct pack_info *const *b = b_;
if (0 <= (*a)->old_num && 0 <= (*b)->old_num)
/* Keep the order in the original */
I certainly won't defend Metrowerks C 2.4.1, but the patch is
harmless enough.
> Have you thought about getting git to compile in C++ mode, where
If the Git maintainers are interested in this, I'm glad to do it. I
expect it to require generally less maintenance than my c++-
initializer-fix branch, which is rather ugly to boot.
> Metrowerks might be a little more sane[1]? Sure, this runs into
Well, I wasn't going to suggest general C++ compatibility just for my
port, but if there's interest, then yes, that would obviate my (by
far) largest and nastiest patch.
> basically all the major incompatibilities between C and C++[2], but
> that might not be insurmountable:
Writing code which is both valid C and valid C++ is not difficult.
> . No implicit conversion from void* to other: don’t use void *,
> then. With type-safe interfaces like
>
> #define typed_malloc(size, type) (type *)xmalloc(size)
> #define malloc_many(nmemb, type) typed_malloc((nmemb) * sizeof
> (type), type)
>
> one can take advantage of type checking without the annoyance of
> casts at the call site.
Replacing xmalloc(size) with typed_malloc(size, struct foo) instead
of (struct foo *)xmalloc(size) still feels like a cast to me. IMHO
it adds no safety and reduces readability.
Maybe you could have:
#define alloc_n(type, n) ((type*)xmalloc(sizeof (type) * n))
to factor the multiplication out of the calling code. But for a
plain data buffer, nothing is shorter or clearer than a simple
(char*) cast.
> Another place git uses void * is for low-level access to the
> object database, because it is not obvious whether objects data
> should use char * or unsigned char *. unsigned char * should
> be fine.
If there were a configurable typedef defining this, I'd use char*
since the Metrowerks debugger by default displays it as a C string
(as opposed to a Pascal string for unsigned char* -- it's a Mac OS
convention). But that's a minor point and not one I'd spend time
advocating.
> . Use of C++ keywords:
>
> #ifdef __cplusplus
> #define template git_template
> #define typename git_typename
> #endif
>
> It is not like this is an actual C++ program.
My c++initializer-fix patch does this locally in a few places. I
suppose it makes sense to nail them all at once in git-compat-util.h.
> . Assignment of ints to enums is forbidden: okay, this one is
> not worth working around. Does Metrowerks have an option to turn
> off this piece of C++ insanity?
It's a feature. :-) I use enums for type-safety in my C++ POSIX
wrapper library. p7::wait() returns p7::wait_t, and if you try to
pass that to p7::exit() (which takes p7::exit_t) you get a compile
error (instead of a program that 'works' when the wait status is zero).
(No, I'm not aware of an option to disable this restriction.)
If you want the type-safety, you have to pay for it. Otherwise,
declare variables and parameters as integral types instead. I don't
care one way or the other -- I just want git to work on Lamp, and if
there's community interest in C++ compatibility then I can assist
with that.
Josh
P.S. Every patch in my git repository is Signed-off-by: Joshua Juran
<jjuran@gmail.com> (or s/gmail/metamage/).
