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

PATCH In review Haritha via GitGitGadget 6 participants 18 messages Jan 31, 2024 – Mar 6, 2024

lore

Share & tools

lore

Haritha via GitGitGadget

author

2024-01-31 14:21 UTC #0 lore raw

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

From: Haritha D <[email protected]>

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 <[email protected]>

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

config.mak.uname +12 -0

index dacc95172dc..c8006f854e5 100644
@@ -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")

configure.ac +3 -0

index d1a96da14eb..64569a80d53 100644
@@ -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

Signature · 1 line

gitgitgadget

Kristoffer Haugsbakk

2024-01-31 16:12 UTC #1 lore raw

Quoted 1 line

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

Maybe the subject could be:

Quoted 1 line

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

Signature · 1 line

Kristoffer Haugsbakk

Junio C Hamano

2024-01-31 17:12 UTC #3 lore raw

In reply to Haritha via GitGitGadget

Quoted 1 line · Haritha via GitGitGadget

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.

Quoted 7 lines

From: Haritha D <[email protected]>
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.

Quoted 7 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 <[email protected]>
---
This PR enables a successful git build on z/OS.

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

Quoted 10 lines

config.mak.uname +2 -0

index dacc95172dc..c8006f854e5 100644
@@ -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

config.mak.uname ifeq ($(uname_S),NONSTOP_KERNEL) · line 652

Quoted 10 lines

config.mak.uname +10 -0

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

Quoted 5 lines

configure.ac +0 -0

index d1a96da14eb..64569a80d53 100644

configure.ac else · line 471

Quoted 9 lines

configure.ac +3 -0

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

Junio C Hamano

2024-01-31 17:58 UTC #2 lore raw

In reply to Kristoffer Haugsbakk

Quoted 7 lines · Kristoffer Haugsbakk

Hi
Quoted 1 line
[PATCH] This PR enables a successful git build on z/OS.
Maybe the subject could be:
Quoted 1 line
[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.

Haritha via GitGitGadget

author

2024-02-23 03:48 UTC #4 lore raw

v2 cover This PR enables a successful git build on z/OS.

In reply to 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 ([email protected]) 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 <[email protected]>

## 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 <[email protected]>

-: ----------- > 2: 05df5d7e2d5 an improvement: removed configure.ac changes

Signature · 1 line

gitgitgadget

Haritha D via GitGitGadget

author

2024-02-23 03:48 UTC #5 lore raw

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

From: Haritha D <[email protected]>

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 <[email protected]>

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

config.mak.uname +12 -0

index dacc95172dc..c8006f854e5 100644
@@ -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")

configure.ac +3 -0

index d1a96da14eb..64569a80d53 100644
@@ -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

Signature · 1 line

gitgitgadget

Haritha D via GitGitGadget

author

2024-02-23 03:48 UTC #6 lore raw

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

In reply to Haritha via GitGitGadget

From: Haritha D <[email protected]>

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 <[email protected]>

---
 configure.ac | 3 ---
 1 file changed, 3 deletions(-)

configure.ac +0 -3

index 64569a80d53..d1a96da14eb 100644
@@ -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

Signature · 1 line

gitgitgadget

Junio C Hamano

2024-02-23 07:38 UTC #8 lore raw

In reply to Haritha via GitGitGadget

Quoted 5 lines · 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.

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

Quoted 2 lines

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

Quoted 8 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 ([email protected]) 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?

Junio C Hamano

2024-02-23 07:38 UTC #7 lore raw

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

In reply to Haritha D via GitGitGadget

Quoted 11 lines · Haritha D via GitGitGadget

From: Haritha D <[email protected]>
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 <[email protected]>
---
 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.

Quoted 14 lines

configure.ac +0 -3

index 64569a80d53..d1a96da14eb 100644
@@ -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

Haritha via GitGitGadget

author

2024-02-25 06:10 UTC #9 lore raw

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

In reply to Haritha via GitGitGadget

From: Haritha D <[email protected]>

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 <[email protected]>

---
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 ([email protected]) 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(+)

config.mak.uname +12 -0

index dacc95172dc..03ee2b74525 100644
@@ -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

Signature · 1 line

gitgitgadget

Junio C Hamano

2024-02-26 17:30 UTC #10 lore raw

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

Quoted 8 lines · Haritha via GitGitGadget

From: Haritha D <[email protected]>
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?

Quoted 1 line

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

Quoted 9 lines

config.mak.uname +1 -0

index dacc95172dc..03ee2b74525 100644
@@ -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)

config.mak.uname ifeq ($(uname_S),NONSTOP_KERNEL) · line 651

Quoted 10 lines

config.mak.uname +10 -0

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

Quoted 6 lines

config.mak.uname +1 -0
+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

2024-03-01 09:09 UTC #11 lore raw

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" <[email protected] <mailto:[email protected]>> wrote:

Quoted 8 lines · "Haritha via GitGitGadget" <[email protected]…

From: Haritha D <[email protected] <mailto:[email protected]>>
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.

Quoted 1 line

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

Quoted 11 lines

config.mak.uname +0 -0
index dacc95172dc..03ee2b74525 100644
@@ -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

Quoted 8 lines

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

Quoted 6 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

2024-03-01 14:39 UTC #12 lore raw

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

Quoted 5 lines · 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.

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

2024-03-01 18:15 UTC #13 lore raw

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

In reply to Haritha D

Quoted 3 lines · Haritha D

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.

[email protected]

2024-03-01 18:25 UTC #14 lore raw

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

Quoted 7 lines · Junio C Hamano

Quoted 3 lines · Haritha D
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

Quoted 3 lines

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

Quoted 7 lines

machinery at all.
Thanks.
[Footnotes]

Quoted 7 lines

*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

2024-03-04 04:19 UTC #15 lore raw

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, "[email protected] <mailto:[email protected]>" <[email protected] <mailto:[email protected]>> wrote:

Quoted 7 lines · Junio C Hamano

Quoted 3 lines · Haritha D <[email protected] <mailto:[email protected]>> 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

Quoted 3 lines

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

Quoted 7 lines

machinery at all.
Thanks.
[Footnotes]

Quoted 7 lines

*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

author

2024-03-06 05:44 UTC #16 lore raw

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

In reply to Haritha via GitGitGadget

From: Haritha D <[email protected]>

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

Signed-off-by Haritha D <[email protected]>

---
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 <[email protected]>

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

config.mak.uname +12 -0

index dacc95172dc..d0dcca2ec55 100644
@@ -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

Signature · 1 line

gitgitgadget

Junio C Hamano

2024-03-06 16:10 UTC #17 lore raw

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

Quoted 7 lines · Haritha via GitGitGadget

From: Haritha D <[email protected]>
Introduced z/OS (OS/390) as a platform in config.mak.uname
Signed-off-by Haritha D <[email protected]>
---
This PR enables a successful git build on z/OS.

Good.

Quoted 13 lines

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

config.mak.uname +2 -0

index dacc95172dc..d0dcca2ec55 100644
@@ -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

config.mak.uname ifeq ($(uname_S),NONSTOP_KERNEL) · line 652

Quoted 10 lines

config.mak.uname +10 -0

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