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

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

[Haritha via GitGitGadget]

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(+)

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

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

[Kristoffer Haugsbakk]

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

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

[Junio C Hamano]

"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.

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

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

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

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

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

[Junio C Hamano]

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

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

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

[Haritha via GitGitGadget]

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!

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

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

[Haritha D via GitGitGadget]

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(+)

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

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

[Haritha D via GitGitGadget]

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(-)

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

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

[Junio C Hamano]

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

> 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)?

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

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

[Junio C Hamano]

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

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

> 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

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

[Haritha via GitGitGadget]

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(+)

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

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

[Junio C Hamano]

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

> 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).

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

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

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

[Haritha D]

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:


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


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).


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


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

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

[Ghanshyam Thakkar]

On Fri Mar 1, 2024 at 2:39 PM IST, Haritha D wrote:
> 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.

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

[Junio C Hamano]

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.

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

[rsbecker]

On Friday, March 1, 2024 1:15 PM, Junio C Hamano wrote:
>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.

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

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

[Haritha D]

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

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

[Haritha via GitGitGadget]

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(+)

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

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

[Junio C Hamano]

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

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

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