"Haritha via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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@xxxxxxx>
>
> 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@xxxxxxx>
> ---
> 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?
