threads / patch / 60823

patchThis PR enables a successful git build on z/OS.

Subject: [PATCH] This PR enables a successful git build on z/OS.

## tl;dr

18 messages between Jan 31, 2024 and Mar 6, 2024. Diffs are folded; open one to read it.

replies: 17people: 6as markdown or json

Haritha via GitGitGadget· Jan 31, 2024, 14:21 UTC · lore

From: Haritha D <harithamma.d@ibm.com>

Since the z/OS linker does not support searching dynamic libraries, and the current setting of CC_LD_DYNPATH results in a directory to be supplied to the link step with no option as the suffix, it causes a linker error because the z/OS LD linker does not accept directories as input. Therefore, we workaround this by adding the -L option. And, Introduced z/OS (OS/390) as a platform in config.mak.uname

Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
    This PR enables a successful git build on z/OS.
    
    Since the z/OS linker does not support searching dynamic libraries, and
    the current setting of CC_LD_DYNPATH results in a directory to be
    supplied to the link step with no option as the suffix, it causes a
    linker error because the z/OS LD linker does not accept directories as
    input. Therefore, we workaround this by adding the -L option. And,
    Introduced z/OS (OS/390) as a platform in config.mak.uname

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1663%2FHarithaIBM%2Fzos-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1663/HarithaIBM/zos-v1
Pull-Request: https://github.com/git/git/pull/1663

 config.mak.uname | 12 ++++++++++++
 configure.ac     |  3 +++
 2 files changed, 15 insertions(+)

Show changes to 2 files +15 −0

config.mak.uname, configure.ac

diff --git a/config.mak.uname b/config.mak.uname
index dacc95172dc..c8006f854e5 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -638,6 +638,18 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
 	SHELL_PATH = /usr/coreutils/bin/bash
 endif
+ifeq ($(uname_S),OS/390)
+        NO_SYS_POLL_H = YesPlease
+        NO_STRCASESTR = YesPlease
+        NO_REGEX = YesPlease
+        NO_MMAP = YesPlease
+        NO_NSEC = YesPlease
+        NO_STRLCPY = YesPlease
+        NO_MEMMEM = YesPlease
+        NO_GECOS_IN_PWENT = YesPlease
+        HAVE_STRINGS_H = YesPlease
+        NEEDS_MODE_TRANSLATION = YesPlease
+endif
 ifeq ($(uname_S),MINGW)
 	ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
 		$(error "Building with MSys is no longer supported")
diff --git a/configure.ac b/configure.ac
index d1a96da14eb..64569a80d53 100644
--- a/configure.ac
+++ b/configure.ac
@@ -463,6 +463,9 @@ else
             CC_LD_DYNPATH=-Wl,+b,
           else
              CC_LD_DYNPATH=
+	     if test "$(uname -s)" = "OS/390"; then
+		     CC_LD_DYNPATH=-L
+	     fi
              AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
           fi
       fi

base-commit: bc7ee2e5e16f0d1e710ef8fab3db59ab11f2bbe7

-- 
gitgitgadget

Kristoffer Haugsbakk· Jan 31, 2024, 16:12 UTC · re: Haritha via GitGitGadget · lore

Re: [PATCH] This PR enables a successful git build on z/OS.

Hi

> [PATCH] This PR enables a successful git build on z/OS.

Maybe the subject could be:

> [PATCH] build: support z/OS (OS/390)

There’s some indirectly related things in `SubmittingPatches`:

 “  Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
    instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
    to do frotz", as if you are giving orders to the codebase to change
    its behavior.  Try to make sure your explanation can be understood
    without external resources. Instead of giving a URL to a mailing
    list archive, summarize the relevant points of the discussion.

From which I infer that “This PR” is considered redundant.

As for the `build:` prefix: many different prefixes have been used for this file (`git log --no-merges -- config.mak.uname`).

-- 
Kristoffer Haugsbakk

Junio C Hamano· Jan 31, 2024, 17:58 UTC · re: Kristoffer Haugsbakk · lore

Re: [PATCH] This PR enables a successful git build on z/OS.

"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:

Show 7 quoted lines

> Hi
>
>> [PATCH] This PR enables a successful git build on z/OS.
>
> Maybe the subject could be:
>
>> [PATCH] build: support z/OS (OS/390)

Nice. This is nicer than what I came up with, that placed too much stress on the runtime path to the dynamic library. With all the stuff added to config.mak.uname, yours is much more appropriate.

Thanks.

Junio C Hamano· Jan 31, 2024, 17:12 UTC · re: Haritha via GitGitGadget · lore

Re: [PATCH] This PR enables a successful git build on z/OS.

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

> Subject: Re: [PATCH] This PR enables a successful git build on z/OS.

Please think with longer term effects in mind when formulating the title of the commit. What title will your next patch have if we break the build for z/OS next time, after this fix goes in? "enables a successful build again"? How would one tell which commit changed what aspect of the build procedure to adjust to z/OS?

Perhaps

    [PATCH] Makefile: adjust for z/OS that lack dynamic library support

or something would be specific enough.

Show 7 quoted lines

> From: Haritha D <harithamma.d@ibm.com>
>
> Since the z/OS linker does not support searching dynamic libraries,
> and the current setting of CC_LD_DYNPATH results in a directory
> to be supplied to the link step with no option as the suffix,
> it causes a linker error because the z/OS LD linker
> does not accept directories as input.

Hmph, it is not quite clear to me where that "current setting of CC_LD_DYNPATH" comes from and what exact value it is set to.

Here is my attempt (blind guesses are involved, so please correct whatever errors you spot):

    The autoconf generated configuration gives an empty string to
    CC_LD_DYNPATH when it cannot find a way to use a shared library
    and gives up with "linker does not support runtime path to
    dynamic libraries" message.  This leaves the directory path that
    is usually appended to -Wl,-rpath, or -R, or whatever alone on
    the command line of the linker, e.g. "-L/usr/lib /usr/lib", which
    breaks the linker.

    Work it around by setting CD_LD_DYNPATH to -L; we will end up
    giving the same directory twice, e.g., "-L/usr/lib -L/usr/lib",
    but it is only ugly without breaking anything.

    While at it, define appropriate settings for z/OS (OS/390) in
    the config.mak.uname file.

Show 13 quoted lines

> Therefore, we workaround this by adding the -L option.
> And, Introduced z/OS (OS/390) as a platform in config.mak.uname
>
> Signed-off-by: Haritha D <harithamma.d@ibm.com>
> ---
>     This PR enables a successful git build on z/OS.
>     
>     Since the z/OS linker does not support searching dynamic libraries, and
>     the current setting of CC_LD_DYNPATH results in a directory to be
>     supplied to the link step with no option as the suffix, it causes a
>     linker error because the z/OS LD linker does not accept directories as
>     input. Therefore, we workaround this by adding the -L option. And,
>     Introduced z/OS (OS/390) as a platform in config.mak.uname

You do not have to write the same thing twice. The text under "---" is meant for extra explanation that does not need to become part of the final commit log message.

Show 20 quoted lines

> diff --git a/config.mak.uname b/config.mak.uname
> index dacc95172dc..c8006f854e5 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -638,6 +638,18 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
>  	SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
>  	SHELL_PATH = /usr/coreutils/bin/bash
>  endif
> +ifeq ($(uname_S),OS/390)
> +        NO_SYS_POLL_H = YesPlease
> +        NO_STRCASESTR = YesPlease
> +        NO_REGEX = YesPlease
> +        NO_MMAP = YesPlease
> +        NO_NSEC = YesPlease
> +        NO_STRLCPY = YesPlease
> +        NO_MEMMEM = YesPlease
> +        NO_GECOS_IN_PWENT = YesPlease
> +        HAVE_STRINGS_H = YesPlease
> +        NEEDS_MODE_TRANSLATION = YesPlease
> +endif

I cannot tell if these are reasonable for z/OS myself and I'll take your word for it ;-) After all you're the expert.

Show 14 quoted lines

> diff --git a/configure.ac b/configure.ac
> index d1a96da14eb..64569a80d53 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -463,6 +463,9 @@ else
>              CC_LD_DYNPATH=-Wl,+b,
>            else
>               CC_LD_DYNPATH=
> +	     if test "$(uname -s)" = "OS/390"; then
> +		     CC_LD_DYNPATH=-L
> +	     fi
>               AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
>            fi
>        fi

The use of "uname -s" looks totally out of place.

Wouldn't it be a better approach to set it in config.mak.uname for OS/390 above and leave this part untouched, I wonder?

Haritha via GitGitGadget· Feb 23, 2024, 03:48 UTC · re: Haritha via GitGitGadget · lore

[PATCH v2 0/2] This PR enables a successful git build on z/OS.

Thanks for taking the time to contribute to Git! Please be advised that the Git community does not use github.com for their contributions. Instead, we use a mailing list (git@vger.kernel.org) for code submissions, code reviews, and bug reports. Nevertheless, you can use GitGitGadget (https://gitgitgadget.github.io/) to conveniently send your Pull Requests commits to our mailing list.

Please read the "guidelines for contributing" linked above!

Haritha D (2):
  build: support z/OS (OS/390).
  an improvement: removed configure.ac changes

 config.mak.uname | 12 ++++++++++++
 1 file changed, 12 insertions(+)

base-commit: f41f85c9ec8d4d46de0fd5fded88db94d3ec8c11
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1663%2FHarithaIBM%2Fzos-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1663/HarithaIBM/zos-v2
Pull-Request: https://github.com/git/git/pull/1663

Range-diff vs v1:

 1:  2eddc8f8860 ! 1:  53e211d7a65 This PR enables a successful git build on z/OS.
     @@ Metadata
      Author: Haritha D <harithamma.d@ibm.com>
      
       ## Commit message ##
     -    This PR enables a successful git build on z/OS.
     +    build: support z/OS (OS/390).
      
          Since the z/OS linker does not support searching dynamic libraries,
          and the current setting of CC_LD_DYNPATH results in a directory
          to be supplied to the link step with no option as the suffix,
          it causes a linker error because the z/OS LD linker
          does not accept directories as input.
     -    Therefore, we workaround this by adding the -L option.
     -    And, Introduced z/OS (OS/390) as a platform in config.mak.uname
     +    Therefore, -L option is added.
     +    Also introduced z/OS (OS/390) as a platform in config.mak.uname
      
          Signed-off-by: Haritha D <harithamma.d@ibm.com>
      
 -:  ----------- > 2:  05df5d7e2d5 an improvement: removed configure.ac changes

-- 
gitgitgadget

Haritha D via GitGitGadget· Feb 23, 2024, 03:48 UTC · re: Haritha via GitGitGadget · lore

[PATCH v2 1/2] build: support z/OS (OS/390).

From: Haritha D <harithamma.d@ibm.com>

Since the z/OS linker does not support searching dynamic libraries, and the current setting of CC_LD_DYNPATH results in a directory to be supplied to the link step with no option as the suffix, it causes a linker error because the z/OS LD linker does not accept directories as input. Therefore, -L option is added. Also introduced z/OS (OS/390) as a platform in config.mak.uname

Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
 config.mak.uname | 12 ++++++++++++
 configure.ac     |  3 +++
 2 files changed, 15 insertions(+)

Show changes to 2 files +15 −0

config.mak.uname, configure.ac

diff --git a/config.mak.uname b/config.mak.uname
index dacc95172dc..c8006f854e5 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -638,6 +638,18 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
 	SHELL_PATH = /usr/coreutils/bin/bash
 endif
+ifeq ($(uname_S),OS/390)
+        NO_SYS_POLL_H = YesPlease
+        NO_STRCASESTR = YesPlease
+        NO_REGEX = YesPlease
+        NO_MMAP = YesPlease
+        NO_NSEC = YesPlease
+        NO_STRLCPY = YesPlease
+        NO_MEMMEM = YesPlease
+        NO_GECOS_IN_PWENT = YesPlease
+        HAVE_STRINGS_H = YesPlease
+        NEEDS_MODE_TRANSLATION = YesPlease
+endif
 ifeq ($(uname_S),MINGW)
 	ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
 		$(error "Building with MSys is no longer supported")
diff --git a/configure.ac b/configure.ac
index d1a96da14eb..64569a80d53 100644
--- a/configure.ac
+++ b/configure.ac
@@ -463,6 +463,9 @@ else
             CC_LD_DYNPATH=-Wl,+b,
           else
              CC_LD_DYNPATH=
+	     if test "$(uname -s)" = "OS/390"; then
+		     CC_LD_DYNPATH=-L
+	     fi
              AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
           fi
       fi

-- 
gitgitgadget

Haritha D via GitGitGadget· Feb 23, 2024, 03:48 UTC · re: Haritha via GitGitGadget · lore

[PATCH v2 2/2] an improvement: removed configure.ac changes

From: Haritha D <harithamma.d@ibm.com>

Hello Reviewers, as an improvement, I have removed the CC_LD_DYNPATH changes because I was able to achieve the desired outcome by passing it to the make step.

Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
 configure.ac | 3 ---
 1 file changed, 3 deletions(-)

Show changes to configure.ac +0 −3

diff --git a/configure.ac b/configure.ac
index 64569a80d53..d1a96da14eb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -463,9 +463,6 @@ else
             CC_LD_DYNPATH=-Wl,+b,
           else
              CC_LD_DYNPATH=
-	     if test "$(uname -s)" = "OS/390"; then
-		     CC_LD_DYNPATH=-L
-	     fi
              AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
           fi
       fi

-- 
gitgitgadget

Junio C Hamano· Feb 23, 2024, 07:38 UTC · re: Haritha D via GitGitGadget · lore

Re: [PATCH v2 2/2] an improvement: removed configure.ac changes

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

Show 11 quoted lines

> From: Haritha D <harithamma.d@ibm.com>
>
> Hello Reviewers, as an improvement, I have
> removed the CC_LD_DYNPATH changes because
> I was able to achieve the desired outcome
> by passing it to the make step.
>
> Signed-off-by: Haritha D <harithamma.d@ibm.com>
> ---
>  configure.ac | 3 ---
>  1 file changed, 3 deletions(-)

We frown upon a patch series that makes mistakes in an earlier step, only to fix them in a later step. The "git rebase -i" command helps us pretend to be more perfect developers than we actually are, whipping your patch series into a shape that builds one small step on top of another in a logical succession. Such a patch series is easier to understand than a history that faithfully records all the stumbles the developer made until they reached the final solution.

In this case, if you know before you sent these two patches that you do not need to touch configure.ac for CC_LD_DYNPATH at all, you can "git rebase -i" to whip the first patch into the desired shape, i.e. no changes to configure.ac, and drop this second patch, I think.

Show 14 quoted lines

> diff --git a/configure.ac b/configure.ac
> index 64569a80d53..d1a96da14eb 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -463,9 +463,6 @@ else
>              CC_LD_DYNPATH=-Wl,+b,
>            else
>               CC_LD_DYNPATH=
> -	     if test "$(uname -s)" = "OS/390"; then
> -		     CC_LD_DYNPATH=-L
> -	     fi
>               AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
>            fi
>        fi

Junio C Hamano· Feb 23, 2024, 07:37 UTC · re: Haritha via GitGitGadget · lore

Re: [PATCH v2 0/2] This PR enables a successful git build on z/OS.

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

Show 5 quoted lines

> Since the z/OS linker does not support searching dynamic libraries, and the
> current setting of CC_LD_DYNPATH results in a directory to be supplied to
> the link step with no option as the suffix, it causes a linker error because
> the z/OS LD linker does not accept directories as input. Therefore, we
> workaround this by adding the -L option.

Hmph, do we still need that CC_LD_DYNPATH hack, with 07bbe4ca (Merge branch 'jc/make-libpath-template', 2024-02-06) already in 'master'?

> And, Introduced z/OS (OS/390) as a
> platform in config.mak.uname

	Add support for z/OS (OS/390) in config.mak.uname.

or something (cf. Documentation/SubmittingPatches)?

Show 8 quoted lines

> Thanks for taking the time to contribute to Git! Please be advised that the
> Git community does not use github.com for their contributions. Instead, we
> use a mailing list (git@vger.kernel.org) for code submissions, code reviews,
> and bug reports. Nevertheless, you can use GitGitGadget
> (https://gitgitgadget.github.io/) to conveniently send your Pull Requests
> commits to our mailing list.
>
> Please read the "guidelines for contributing" linked above!

For whom is the above message addressed? Who is expected to read that guidelines?

Haritha via GitGitGadget· Feb 25, 2024, 06:10 UTC · re: Haritha via GitGitGadget · lore

[PATCH v3] build: support z/OS (OS/390).

From: Haritha D <harithamma.d@ibm.com>

Since the z/OS linker does not support searching dynamic libraries, and the current setting of CC_LD_DYNPATH results in a directory to be supplied to the link step with no option as the suffix, it causes a linker error because the z/OS LD linker does not accept directories as input. Therefore, -L option is added. Also introduced z/OS (OS/390) as a platform in config.mak.uname

Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
    This PR enables a successful git build on z/OS.
    
    Since the z/OS linker does not support searching dynamic libraries, and
    the current setting of CC_LD_DYNPATH results in a directory to be
    supplied to the link step with no option as the suffix, it causes a
    linker error because the z/OS LD linker does not accept directories as
    input. Therefore, we workaround this by adding the -L option. And,
    Introduced z/OS (OS/390) as a platform in config.mak.uname
    
    Thanks for taking the time to contribute to Git! Please be advised that
    the Git community does not use github.com for their contributions.
    Instead, we use a mailing list (git@vger.kernel.org) for code
    submissions, code reviews, and bug reports. Nevertheless, you can use
    GitGitGadget (https://gitgitgadget.github.io/) to conveniently send your
    Pull Requests commits to our mailing list.
    
    Please read the "guidelines for contributing" linked above!

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1663%2FHarithaIBM%2Fzos-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1663/HarithaIBM/zos-v3
Pull-Request: https://github.com/git/git/pull/1663

Range-diff vs v2:

 1:  53e211d7a65 ! 1:  2f1ad41bc14 build: support z/OS (OS/390).
     @@ config.mak.uname: ifeq ($(uname_S),NONSTOP_KERNEL)
      +        NO_MEMMEM = YesPlease
      +        NO_GECOS_IN_PWENT = YesPlease
      +        HAVE_STRINGS_H = YesPlease
     -+        NEEDS_MODE_TRANSLATION = YesPlease
     ++       NEEDS_MODE_TRANSLATION = YesPlease
      +endif
       ifeq ($(uname_S),MINGW)
       	ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
       		$(error "Building with MSys is no longer supported")
     -
     - ## configure.ac ##
     -@@ configure.ac: else
     -             CC_LD_DYNPATH=-Wl,+b,
     -           else
     -              CC_LD_DYNPATH=
     -+	     if test "$(uname -s)" = "OS/390"; then
     -+		     CC_LD_DYNPATH=-L
     -+	     fi
     -              AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
     -           fi
     -       fi
 2:  05df5d7e2d5 < -:  ----------- an improvement: removed configure.ac changes

 config.mak.uname | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Show changes to config.mak.uname +12 −0

diff --git a/config.mak.uname b/config.mak.uname
index dacc95172dc..03ee2b74525 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -638,6 +638,18 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
 	SHELL_PATH = /usr/coreutils/bin/bash
 endif
+ifeq ($(uname_S),OS/390)
+        NO_SYS_POLL_H = YesPlease
+        NO_STRCASESTR = YesPlease
+        NO_REGEX = YesPlease
+        NO_MMAP = YesPlease
+        NO_NSEC = YesPlease
+        NO_STRLCPY = YesPlease
+        NO_MEMMEM = YesPlease
+        NO_GECOS_IN_PWENT = YesPlease
+        HAVE_STRINGS_H = YesPlease
+       NEEDS_MODE_TRANSLATION = YesPlease
+endif
 ifeq ($(uname_S),MINGW)
 	ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
 		$(error "Building with MSys is no longer supported")

base-commit: f41f85c9ec8d4d46de0fd5fded88db94d3ec8c11

-- 
gitgitgadget

Junio C Hamano· Feb 26, 2024, 17:30 UTC · re: Haritha via GitGitGadget · lore

Re: [PATCH v3] build: support z/OS (OS/390).

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

Show 8 quoted lines

> From: Haritha D <harithamma.d@ibm.com>
>
> Since the z/OS linker does not support searching dynamic libraries,
> and the current setting of CC_LD_DYNPATH results in a directory
> to be supplied to the link step with no option as the suffix,
> it causes a linker error because the z/OS LD linker
> does not accept directories as input.
> Therefore, -L option is added.

Shouldn't all of the above removed by now, with 07bbe4ca (Merge branch 'jc/make-libpath-template', 2024-02-06) that merged the support for platforms whose linker does not have a way to specify the path used at runtime to locate dynamic libraries and is already in the Git 2.44 release? The autoconf-generated config.mak.autogen would leave CC_LD_DYNPATH to empty on such a platform, which then adds only one "-L $where" (instead of the usual "-L $where -R $where") on the linker command line, making your earlier workaround to use "-L $where -L $where" unnecessary?

> Also introduced z/OS (OS/390) as a platform in config.mak.uname

"Introduce z/OS ..." (cf. Documentation/SubmittingPatches)

Or perhaps

	Add platform specific settings in config.mak.uname for zOS
	(OS/390).

Show 19 quoted lines

> diff --git a/config.mak.uname b/config.mak.uname
> index dacc95172dc..03ee2b74525 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -638,6 +638,18 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
>  	SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
>  	SHELL_PATH = /usr/coreutils/bin/bash
>  endif
> +ifeq ($(uname_S),OS/390)
> +        NO_SYS_POLL_H = YesPlease
> +        NO_STRCASESTR = YesPlease
> +        NO_REGEX = YesPlease
> +        NO_MMAP = YesPlease
> +        NO_NSEC = YesPlease
> +        NO_STRLCPY = YesPlease
> +        NO_MEMMEM = YesPlease
> +        NO_GECOS_IN_PWENT = YesPlease
> +        HAVE_STRINGS_H = YesPlease
> +       NEEDS_MODE_TRANSLATION = YesPlease

All of the above should be indented with TAB to imitate the support for other platforms, I think.

Thanks.

Show 6 quoted lines

> +endif
>  ifeq ($(uname_S),MINGW)
>  	ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
>  		$(error "Building with MSys is no longer supported")
>
> base-commit: f41f85c9ec8d4d46de0fd5fded88db94d3ec8c11

Haritha D· Mar 1, 2024, 09:09 UTC · re: Junio C Hamano · lore

RE: [PATCH v3] build: support z/OS (OS/390).

Hi Team,

The win test(7) test case failed stating the reason as "The Operation cancelled". I saw that it failed after 5 hours 59 minutes of running the test cases(build). How do I handle this?

Please suggest.

Best regards Haritha

On 26/02/24, 11:15 PM, "Junio C Hamano" <gitster@pobox.com <mailto:gitster@pobox.com>> wrote:

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

Show 8 quoted lines

> From: Haritha D <harithamma.d@ibm.com <mailto:harithamma.d@ibm.com>>
>
> Since the z/OS linker does not support searching dynamic libraries,
> and the current setting of CC_LD_DYNPATH results in a directory
> to be supplied to the link step with no option as the suffix,
> it causes a linker error because the z/OS LD linker
> does not accept directories as input.
> Therefore, -L option is added.

> Also introduced z/OS (OS/390) as a platform in config.mak.uname

"Introduce z/OS ..." (cf. Documentation/SubmittingPatches)

Or perhaps

Add platform specific settings in config.mak.uname for zOS (OS/390).

Show 19 quoted lines

> diff --git a/config.mak.uname b/config.mak.uname
> index dacc95172dc..03ee2b74525 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -638,6 +638,18 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
> SHELL_PATH = /usr/coreutils/bin/bash
> endif
> +ifeq ($(uname_S),OS/390)
> + NO_SYS_POLL_H = YesPlease
> + NO_STRCASESTR = YesPlease
> + NO_REGEX = YesPlease
> + NO_MMAP = YesPlease
> + NO_NSEC = YesPlease
> + NO_STRLCPY = YesPlease
> + NO_MEMMEM = YesPlease
> + NO_GECOS_IN_PWENT = YesPlease
> + HAVE_STRINGS_H = YesPlease
> + NEEDS_MODE_TRANSLATION = YesPlease

All of the above should be indented with TAB to imitate the support for other platforms, I think.

Thanks.

Show 6 quoted lines

> +endif
> ifeq ($(uname_S),MINGW)
> ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
> $(error "Building with MSys is no longer supported")
>
> base-commit: f41f85c9ec8d4d46de0fd5fded88db94d3ec8c11

Ghanshyam Thakkar· Mar 1, 2024, 14:39 UTC · re: Haritha D · lore

Re: [PATCH v3] build: support z/OS (OS/390).

On Fri Mar 1, 2024 at 2:39 PM IST, Haritha D wrote:

Show 5 quoted lines

> Hi Team,
>
> The win test(7) test case failed stating the reason as "The Operation cancelled". I saw that it failed after 5 hours 59 minutes of running the test cases(build). How do I handle this?
>
> Please suggest.

Hey,

This seems to be a recurring issue, as this happens to me majority of the time. Seems as though the vm just sits idle after finishing tests as 360 minutes is the timeout for github acitons.

Usually, rerunning the failed tests from the actions tab works for me.

Thanks.

Junio C Hamano· Mar 1, 2024, 18:15 UTC · re: Haritha D · lore

Re: [PATCH v3] build: support z/OS (OS/390).

Haritha D <Harithamma.D@ibm.com> writes:

> The win test(7) test case failed stating the reason as "The
> Operation cancelled". I saw that it failed after 5 hours 59
> minutes of running the test cases(build). How do I handle this?

Those "win test (n)", at least some of them, seem to have been somewhat flaky [*1*]. If you are certain you did not break them with your change, you do not have to fix them yourself.

I am a wrong person to ask how the test failure that may [*2*] block GGG submission can be circumvented, as I am not involved in that machinery at all.

Thanks.

[Footnotes]

*1* Also I've seen osx-clang job time-out from time to time, without
    failing any specific test.  Re-running failed jobs from the menu
    often make them pass, which is why I said "somewhat flaky".

*2* I do not even know if GGG refuses to submit a series with a test
    failure, let alone if it allows to override such a safety if
    exists.

rsbecker@nexbridge.com· Mar 1, 2024, 18:25 UTC · re: Junio C Hamano · lore

RE: [PATCH v3] build: support z/OS (OS/390).

On Friday, March 1, 2024 1:15 PM, Junio C Hamano wrote:

Show 7 quoted lines

>Haritha D <Harithamma.D@ibm.com> writes:
>
>> The win test(7) test case failed stating the reason as "The Operation
>> cancelled". I saw that it failed after 5 hours 59 minutes of running
>> the test cases(build). How do I handle this?
>
>Those "win test (n)", at least some of them, seem to have been somewhat

flaky [*1*].  If you are certain you did not break them with

>your change, you do not have to fix them yourself.
>
>I am a wrong person to ask how the test failure that may [*2*] block GGG

submission can be circumvented, as I am not involved in that

Show 14 quoted lines

>machinery at all.
>
>Thanks.
>
>
>[Footnotes]
>
>*1* Also I've seen osx-clang job time-out from time to time, without
>    failing any specific test.  Re-running failed jobs from the menu
>    often make them pass, which is why I said "somewhat flaky".
>
>*2* I do not even know if GGG refuses to submit a series with a test
>    failure, let alone if it allows to override such a safety if
>    exists.

Which tests have been hanging on S390? We have occasional hangs on NonStop that end up being attributed to our CI build system not supplying pipes properly to git. It would be interesting if the same tests are having issues on different platforms. --Randall

Haritha D· Mar 4, 2024, 04:19 UTC · re: rsbecker@nexbridge.com · lore

RE: [PATCH v3] build: support z/OS (OS/390).

Looks like , I do not have sufficient permissions to rerun the tests. How do I proceed?

Please suggest.
 
Thank you
Haritha

On 01/03/24, 11:55 PM, "rsbecker@nexbridge.com <mailto:rsbecker@nexbridge.com>" <rsbecker@nexbridge.com <mailto:rsbecker@nexbridge.com>> wrote:

On Friday, March 1, 2024 1:15 PM, Junio C Hamano wrote:

Show 7 quoted lines

>Haritha D <Harithamma.D@ibm.com <mailto:Harithamma.D@ibm.com>> writes:
>
>> The win test(7) test case failed stating the reason as "The Operation
>> cancelled". I saw that it failed after 5 hours 59 minutes of running
>> the test cases(build). How do I handle this?
>
>Those "win test (n)", at least some of them, seem to have been somewhat

flaky [*1*]. If you are certain you did not break them with

>your change, you do not have to fix them yourself.
>
>I am a wrong person to ask how the test failure that may [*2*] block GGG

submission can be circumvented, as I am not involved in that

Show 14 quoted lines

>machinery at all.
>
>Thanks.
>
>
>[Footnotes]
>
>*1* Also I've seen osx-clang job time-out from time to time, without
> failing any specific test. Re-running failed jobs from the menu
> often make them pass, which is why I said "somewhat flaky".
>
>*2* I do not even know if GGG refuses to submit a series with a test
> failure, let alone if it allows to override such a safety if
> exists.

Haritha via GitGitGadget· Mar 6, 2024, 05:44 UTC · re: Haritha via GitGitGadget · lore

[PATCH v4] build: support z/OS (OS/390).

From: Haritha D <harithamma.d@ibm.com>

Introduced z/OS (OS/390) as a platform in config.mak.uname

Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
    This PR enables a successful git build on z/OS.
    
    Introduced z/OS (OS/390) as a platform in config.mak.uname

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1663%2FHarithaIBM%2Fzos-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1663/HarithaIBM/zos-v4
Pull-Request: https://github.com/git/git/pull/1663

Range-diff vs v3:

 1:  2f1ad41bc14 ! 1:  cbc38a801e9 build: support z/OS (OS/390).
     @@ Metadata
       ## Commit message ##
          build: support z/OS (OS/390).
      
     -    Since the z/OS linker does not support searching dynamic libraries,
     -    and the current setting of CC_LD_DYNPATH results in a directory
     -    to be supplied to the link step with no option as the suffix,
     -    it causes a linker error because the z/OS LD linker
     -    does not accept directories as input.
     -    Therefore, -L option is added.
     -    Also introduced z/OS (OS/390) as a platform in config.mak.uname
     +    Introduced z/OS (OS/390) as a platform in config.mak.uname
      
          Signed-off-by: Haritha D <harithamma.d@ibm.com>
      
     @@ config.mak.uname: ifeq ($(uname_S),NONSTOP_KERNEL)
       	SHELL_PATH = /usr/coreutils/bin/bash
       endif
      +ifeq ($(uname_S),OS/390)
     -+        NO_SYS_POLL_H = YesPlease
     -+        NO_STRCASESTR = YesPlease
     -+        NO_REGEX = YesPlease
     -+        NO_MMAP = YesPlease
     -+        NO_NSEC = YesPlease
     -+        NO_STRLCPY = YesPlease
     -+        NO_MEMMEM = YesPlease
     -+        NO_GECOS_IN_PWENT = YesPlease
     -+        HAVE_STRINGS_H = YesPlease
     -+       NEEDS_MODE_TRANSLATION = YesPlease
     ++	NO_SYS_POLL_H = YesPlease
     ++	NO_STRCASESTR = YesPlease
     ++	NO_REGEX = YesPlease
     ++	NO_MMAP = YesPlease
     ++	NO_NSEC = YesPlease
     ++	NO_STRLCPY = YesPlease
     ++	NO_MEMMEM = YesPlease
     ++	NO_GECOS_IN_PWENT = YesPlease
     ++	HAVE_STRINGS_H = YesPlease
     ++	NEEDS_MODE_TRANSLATION = YesPlease
      +endif
       ifeq ($(uname_S),MINGW)
       	ifeq ($(shell expr "$(uname_R)" : '1\.'),2)

 config.mak.uname | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Show changes to config.mak.uname +12 −0

diff --git a/config.mak.uname b/config.mak.uname
index dacc95172dc..d0dcca2ec55 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -638,6 +638,18 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
 	SHELL_PATH = /usr/coreutils/bin/bash
 endif
+ifeq ($(uname_S),OS/390)
+	NO_SYS_POLL_H = YesPlease
+	NO_STRCASESTR = YesPlease
+	NO_REGEX = YesPlease
+	NO_MMAP = YesPlease
+	NO_NSEC = YesPlease
+	NO_STRLCPY = YesPlease
+	NO_MEMMEM = YesPlease
+	NO_GECOS_IN_PWENT = YesPlease
+	HAVE_STRINGS_H = YesPlease
+	NEEDS_MODE_TRANSLATION = YesPlease
+endif
 ifeq ($(uname_S),MINGW)
 	ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
 		$(error "Building with MSys is no longer supported")

base-commit: b387623c12f3f4a376e4d35a610fd3e55d7ea907

-- 
gitgitgadget

Junio C Hamano· Mar 6, 2024, 16:10 UTC · re: Haritha via GitGitGadget · lore

Re: [PATCH v4] build: support z/OS (OS/390).

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

Show 7 quoted lines

> From: Haritha D <harithamma.d@ibm.com>
>
> Introduced z/OS (OS/390) as a platform in config.mak.uname
>
> Signed-off-by: Haritha D <harithamma.d@ibm.com>
> ---
>     This PR enables a successful git build on z/OS.

Good.

Show 23 quoted lines

>  config.mak.uname | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/config.mak.uname b/config.mak.uname
> index dacc95172dc..d0dcca2ec55 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -638,6 +638,18 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
>  	SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
>  	SHELL_PATH = /usr/coreutils/bin/bash
>  endif
> +ifeq ($(uname_S),OS/390)
> +	NO_SYS_POLL_H = YesPlease
> +	NO_STRCASESTR = YesPlease
> +	NO_REGEX = YesPlease
> +	NO_MMAP = YesPlease
> +	NO_NSEC = YesPlease
> +	NO_STRLCPY = YesPlease
> +	NO_MEMMEM = YesPlease
> +	NO_GECOS_IN_PWENT = YesPlease
> +	HAVE_STRINGS_H = YesPlease
> +	NEEDS_MODE_TRANSLATION = YesPlease
> +endif

I somehow expected you to throw in the -L thing in this block, perhaps like

	CC_LD_DYNPATH =

to help those who are on OS/390 but do not run configure (made from configure.ac) to create the config.mak.autogen file, but if you are always building with configure and not testing such a configuration, then doing so and shipping an untested code would not be prudent, so let's accept this patch as-is.

Thanks for working on this.  Will queue.