Re: [PATCH 02/13] Enable builds for z/OS.

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 14, 2023, 00:48 UTC
Message-ID: <xmqqcywd2m9i.fsf@gitster.g>
In-Reply-To: <098b9ca8ece4fdce45a9b48e576b474ed81dced1.1699871056.git.gitgitgadget@gmail.com>

"Haritha D via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Subject: Re: [PATCH 02/13] Enable builds for z/OS.

Documentation/CodingGuidelines and Documentation/SubmittingPatches would help here, I think.

>  # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
>  #
> +# Define SHELL_PATH_FOR_SCRIPTS to a POSIX shell if your /bin/sh is broken.

The reason to exist for the _FOR_SCRIPTS variants is not justified anywhere in the proposed log message.

The former should be sufficient, and our policy is to let the builder specify exactly what binaries the build products depend on, (instead of random $PATH interfere with the choice by using "#!/bin/env tool" that also has to assume that everybody's "env" is installed in "/bin").

This patch has too many #ifdefs in the primary codepaths. Your porting strategy may need to be rethought. Our usual convention is to encapsulate these platform differences as much as possible in git-compat-util.h and platform specific files in compat/ directory.

Show 17 quoted lines

> diff --git a/builtin/hash-object.c b/builtin/hash-object.c
> index 5ffec99dcea..b33b32ff977 100644
> --- a/builtin/hash-object.c
> +++ b/builtin/hash-object.c
> @@ -57,11 +57,39 @@ static void hash_fd(int fd, const char *type, const char *path, unsigned flags,
>  	maybe_flush_or_die(stdout, "hash to stdout");
>  }
>  
> +#ifdef __MVS__
> +#  if (__CHARSET_LIB == 1)
> +#  include <stdio.h>
> +#  include <stdlib.h>
> +
> +   int setbinaryfd(int fd)
> +   {
> +     attrib_t attr;
> +     int rc;

Ahh, OK, I saw [03/13] first and was utterly confused by this thing. Do not send in such a mess that introduces broken code in an early step that you need to later say "oops that one was broken and I need to fix it up with this patch". "rebase -i" is your friend to clean up your mess into a logical progression to help readers better understand what you wrote.

I'll stop here.