From: Joshua Juran Date: Mon, 31 May 2010 04:35:58 GMT Subject: Re: Mac OS 9 (Lamp) port Message-ID: <2776E3E5-234F-4A94-B06A-F4D83D87A414@gmail.com> In-Reply-To: <20100531031906.GA465@progeny.tock> On May 30, 2010, at 8:19 PM, Jonathan Nieder wrote: > Joshua Juran wrote: > >> http://github.com/jjuran/git.git Apparently 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 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. 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 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 (or s/gmail/metamage/).