git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/13] Enabling z/OS workflow for git
@ 2023-11-13 10:24 Haritha  via GitGitGadget
  2023-11-13 10:24 ` [PATCH 01/13] " Haritha D via GitGitGadget
                   ` (13 more replies)
  0 siblings, 14 replies; 21+ messages in thread
From: Haritha  via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha

z/OS is an IBM mainframe operating system, also known as OS/390. Our team
has been actively involved in porting Git to z/OS and we have made
significant modifications to facilitate this process. The patch below is the
initial configuration for z/OS. I also have few follow up changes and I will
send that after these changes are approved. Please let me know if there are
any concerns.

Haritha D (13):
  Enabling z/OS workflow for git
  Enable builds for z/OS.
  spaces and errors fix Handled git pipeline errors
  fixes for build errors Handled git pipeline errorse
  fixes for build errors
  spaces and errors fix Handled git pipeline errors
  spaces and errors fix Handled git pipeline errors
  platform_name fix Handled git pipeline errors
  strncpy fix Handled git pipeline errors
  strncpy fix Handled git pipeline errors
  strncpy fix Handled git pipeline errors
  Handled git pipeline errors - Memory leak
  Handled git pipeline errors - z/OS enable

 Makefile              | 21 +++++++++---
 builtin.h             |  3 ++
 builtin/archive.c     |  6 ++++
 builtin/hash-object.c | 28 +++++++++++++++
 combine-diff.c        |  4 +++
 config.c              |  7 ++++
 config.mak.uname      | 20 +++++++++++
 configure.ac          |  3 ++
 convert.c             | 58 +++++++++++++++++++++++++++----
 copy.c                |  3 ++
 diff.c                | 11 ++++++
 entry.c               | 26 ++++++++++++++
 environment.c         |  3 ++
 fetch-negotiator.h    |  2 +-
 fetch-pack.c          |  4 +--
 git-compat-util.h     |  8 +++++
 negotiator/default.c  |  4 +--
 negotiator/noop.c     |  4 +--
 negotiator/skipping.c |  4 +--
 object-file.c         | 80 ++++++++++++++++++++++++++++++++++++++++++-
 read-cache.c          |  3 ++
 utf8.c                | 11 ++++++
 22 files changed, 292 insertions(+), 21 deletions(-)


base-commit: a9ecda2788e229afc9b611acaa26d0d9d4da53ed
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1537%2FHarithaIBM%2Fenablezos-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1537/HarithaIBM/enablezos-v1
Pull-Request: https://github.com/git/git/pull/1537
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 01/13] Enabling z/OS workflow for git
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 02/13] Enable builds for z/OS Haritha D via GitGitGadget
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

Our team ported Git to z/OS w/ significant modifcation.
This patch is Initial Config

Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
 config.mak.uname | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/config.mak.uname b/config.mak.uname
index 3bb03f423a0..6ba9b707006 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -625,6 +625,26 @@ 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)
+	PERL_PATH = perl
+	PERL_PATH_FOR_SCRIPTS = /bin/env perl
+	SHELL_PATH = bash
+	SHELL_PATH_FOR_SCRIPTS = /bin/env bash
+	PYTHON_PATH = python
+	NO_SYS_POLL_H = YesPlease
+	NO_STRCASESTR = YesPlease
+	NO_REGEX = YesPlease
+	NO_MMAP = YesPlease
+	NO_NSEC = YesPlease
+	NO_STRLCPY = YesPlease
+	NO_MKDTEMP = 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")
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 02/13] Enable builds for z/OS.
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
  2023-11-13 10:24 ` [PATCH 01/13] " Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 23:03   ` brian m. carlson
  2023-11-14  0:48   ` Junio C Hamano
  2023-11-13 10:24 ` [PATCH 03/13] spaces and errors fix Handled git pipeline errors Haritha D via GitGitGadget
                   ` (11 subsequent siblings)
  13 siblings, 2 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This commit enables git to build on z/OS.
It takes advantage of enahanced ASCII
services on z/OS to auto-convert input
files to ASCII
It also adds support for
[platform]-working-tree-encoding.
Platform is substituted with uname_info.sysname,
so it will only apply to the given platform.
Also adds support for scripts that are not in
standard locations so that /bin/env bash
can be specified.

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 Makefile              | 21 +++++++++---
 builtin.h             |  3 ++
 builtin/archive.c     |  6 ++++
 builtin/hash-object.c | 28 +++++++++++++++
 combine-diff.c        |  4 +++
 config.c              |  7 ++++
 configure.ac          |  3 ++
 convert.c             | 44 ++++++++++++++++++++----
 copy.c                |  3 ++
 diff.c                | 11 ++++++
 entry.c               | 26 ++++++++++++++
 environment.c         |  3 ++
 git-compat-util.h     |  8 +++++
 negotiator/default.c  |  4 +--
 negotiator/noop.c     |  4 +--
 negotiator/skipping.c |  4 +--
 object-file.c         | 80 ++++++++++++++++++++++++++++++++++++++++++-
 read-cache.c          |  3 ++
 utf8.c                | 11 ++++++
 19 files changed, 255 insertions(+), 18 deletions(-)

diff --git a/Makefile b/Makefile
index 9c6a2f125f8..30aa76da4f4 100644
--- a/Makefile
+++ b/Makefile
@@ -20,6 +20,8 @@ include shared.mak
 #
 # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
 #
+# Define SHELL_PATH_FOR_SCRIPTS to a POSIX shell if your /bin/sh is broken.
+#
 # Define SANE_TOOL_PATH to a colon-separated list of paths to prepend
 # to PATH if your tools in /usr/bin are broken.
 #
@@ -215,6 +217,8 @@ include shared.mak
 #
 # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl).
 #
+# Define PERL_PATH_FOR_SCRIPTS to a Perl binary if your /usr/bin/perl is broken.
+#
 # Define NO_PERL if you do not want Perl scripts or libraries at all.
 #
 # Define NO_PERL_CPAN_FALLBACKS if you do not want to install bundled
@@ -903,9 +907,15 @@ BINDIR_PROGRAMS_NO_X += git-cvsserver
 ifndef SHELL_PATH
 	SHELL_PATH = /bin/sh
 endif
+ifndef SHELL_PATH_FOR_SCRIPTS
+	SHELL_PATH_FOR_SCRIPTS = /bin/sh
+endif
 ifndef PERL_PATH
 	PERL_PATH = /usr/bin/perl
 endif
+ifndef PERL_PATH_FOR_SCRIPTS
+	PERL_PATH_FOR_SCRIPTS = /usr/bin/perl
+endif
 ifndef PYTHON_PATH
 	PYTHON_PATH = /usr/bin/python
 endif
@@ -1336,7 +1346,7 @@ THIRD_PARTY_SOURCES += sha1dc/%
 
 # xdiff and reftable libs may in turn depend on what is in libgit.a
 GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE)
-EXTLIBS =
+EXTLIBS = $(ZOPEN_EXTRA_LIBS)
 
 GIT_USER_AGENT = git/$(GIT_VERSION)
 
@@ -2226,9 +2236,10 @@ perllibdir_relative_SQ = $(subst ','\'',$(perllibdir_relative))
 gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
 gitwebstaticdir_SQ = $(subst ','\'',$(gitwebstaticdir))
 
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH_FOR_SCRIPTS))
 TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
+PERL_PATH_FOR_SCRIPTS_SQ = $(subst ','\'',$(PERL_PATH_FOR_SCRIPTS))
 PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 DIFF_SQ = $(subst ','\'',$(DIFF))
@@ -2448,7 +2459,7 @@ hook-list.h: generate-hooklist.sh Documentation/githooks.txt
 
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
 	$(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
-	$(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\
+	$(gitwebdir_SQ):$(PERL_PATH_FOR_SCRIPTS_SQ):$(PAGER_ENV):\
 	$(perllibdir_SQ)
 GIT-SCRIPT-DEFINES: FORCE
 	@FLAGS='$(SCRIPT_DEFINES)'; \
@@ -2465,7 +2476,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \
     -e $(BROKEN_PATH_FIX) \
     -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \
-    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
+    -e 's|@@PERL@@|$(PERL_PATH_FOR_SCRIPTS_SQ)|g' \
     -e 's|@@PAGER_ENV@@|$(PAGER_ENV_SQ)|g' \
     $@.sh >$@+
 endef
@@ -2519,7 +2530,7 @@ PERL_DEFINES += $(gitexecdir) $(perllibdir) $(localedir)
 $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
 	$(QUIET_GEN) \
 	sed -e '1{' \
-	    -e '	s|#!.*perl|#!$(PERL_PATH_SQ)|' \
+	    -e '	s|#!.*perl|#!$(PERL_PATH_FOR_SCRIPTS_SQ)|' \
 	    -e '	r GIT-PERL-HEADER' \
 	    -e '	G' \
 	    -e '}' \
diff --git a/builtin.h b/builtin.h
index d560baa6618..806af1a262d 100644
--- a/builtin.h
+++ b/builtin.h
@@ -250,5 +250,8 @@ int cmd_verify_pack(int argc, const char **argv, const char *prefix);
 int cmd_show_ref(int argc, const char **argv, const char *prefix);
 int cmd_pack_refs(int argc, const char **argv, const char *prefix);
 int cmd_replace(int argc, const char **argv, const char *prefix);
+#ifdef __MVS__
+  extern int setbinaryfd(int);
+#endif
 
 #endif
diff --git a/builtin/archive.c b/builtin/archive.c
index 90761fdfee0..53ec794356f 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -14,6 +14,12 @@
 static void create_output_file(const char *output_file)
 {
 	int output_fd = xopen(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
+#ifdef __MVS__
+ #if (__CHARSET_LIB == 1)
+	if (setbinaryfd(output_fd))
+		die_errno(_("could not tag archive file '%s'"), output_file);
+ #endif
+#endif
 	if (output_fd != 1) {
 		if (dup2(output_fd, 1) < 0)
 			die_errno(_("could not redirect output"));
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 5ffec99dcea..b33b32ff977 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -57,11 +57,39 @@ static void hash_fd(int fd, const char *type, const char *path, unsigned flags,
 	maybe_flush_or_die(stdout, "hash to stdout");
 }
 
+#ifdef __MVS__
+#  if (__CHARSET_LIB == 1)
+#  include <stdio.h>
+#  include <stdlib.h>
+
+   int setbinaryfd(int fd)
+   {
+     attrib_t attr;
+     int rc;
+
+     memset(&attr, 0, sizeof(attr));
+     attr.att_filetagchg = 1;
+     attr.att_filetag.ft_ccsid = FT_BINARY;
+     attr.att_filetag.ft_txtflag = 0;
+
+     rc = __fchattr(fd, &attr, sizeof(attr));
+     return rc;
+   }
+#  endif
+#endif
+
+
 static void hash_object(const char *path, const char *type, const char *vpath,
 			unsigned flags, int literally)
 {
 	int fd;
 	fd = xopen(path, O_RDONLY);
+#ifdef __MVS__
+#  if (__CHARSET_LIB == 1)
+  if (setbinaryfd(fd))
+		die_errno("Cannot set to binary '%s'", path);
+#  endif
+#endif
 	hash_fd(fd, type, vpath, flags, literally);
 }
 
diff --git a/combine-diff.c b/combine-diff.c
index f90f4424829..73445a517c7 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1082,6 +1082,10 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			ssize_t done;
 			int is_file, i;
 
+#ifdef __MVS__
+      __disableautocvt(fd);
+#endif
+
 			elem->mode = canon_mode(st.st_mode);
 			/* if symlinks don't work, assume symlink if all parents
 			 * are symlinks
diff --git a/config.c b/config.c
index f9a1cca4e8a..37c124a37c0 100644
--- a/config.c
+++ b/config.c
@@ -1521,6 +1521,13 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
+	#ifdef __MVS__
+	if (!strcmp(var, "core.ignorefiletags")) {
+		ignore_file_tags = git_config_bool(var, value);
+		return 0;
+	}
+	#endif
+
 	if (!strcmp(var, "core.safecrlf")) {
 		int eol_rndtrp_die;
 		if (value && !strcasecmp(value, "warn")) {
diff --git a/configure.ac b/configure.ac
index 276593cd9dd..ed380504be6 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
diff --git a/convert.c b/convert.c
index a8870baff36..4f14ff6f1ed 100644
--- a/convert.c
+++ b/convert.c
@@ -377,12 +377,15 @@ static int check_roundtrip(const char *enc_name)
 static const char *default_encoding = "UTF-8";
 
 static int encode_to_git(const char *path, const char *src, size_t src_len,
-			 struct strbuf *buf, const char *enc, int conv_flags)
+			 struct strbuf *buf, const char *enc, enum convert_crlf_action attr_action, int conv_flags)
 {
 	char *dst;
 	size_t dst_len;
 	int die_on_error = conv_flags & CONV_WRITE_OBJECT;
 
+  if (attr_action == CRLF_BINARY) {
+    return 0;
+  }
 	/*
 	 * No encoding is specified or there is nothing to encode.
 	 * Tell the caller that the content was not modified.
@@ -403,6 +406,11 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,
 		return 0;
 
 	trace_encoding("source", path, enc, src, src_len);
+#ifdef __MVS__
+  // Don't convert ISO8859-1 on z/OS
+  if (strcasecmp("ISO8859-1", enc) == 0)
+    return 0;
+#endif
 	dst = reencode_string_len(src, src_len, default_encoding, enc,
 				  &dst_len);
 	if (!dst) {
@@ -468,11 +476,14 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,
 }
 
 static int encode_to_worktree(const char *path, const char *src, size_t src_len,
-			      struct strbuf *buf, const char *enc)
+			      struct strbuf *buf, enum convert_crlf_action attr_action, const char *enc)
 {
 	char *dst;
 	size_t dst_len;
 
+  if (attr_action == CRLF_BINARY) {
+    return 0;
+  }
 	/*
 	 * No encoding is specified or there is nothing to encode.
 	 * Tell the caller that the content was not modified.
@@ -1302,18 +1313,37 @@ static int git_path_check_ident(struct attr_check_item *check)
 
 static struct attr_check *check;
 
+static const char* get_platform() {
+	struct utsname uname_info;
+
+	if (uname(&uname_info))
+		die(_("uname() failed with error '%s' (%d)\n"),
+			    strerror(errno),
+			    errno);
+
+  if (!strcmp(uname_info.sysname, "OS/390"))
+    return "zos";
+  return uname_info.sysname;
+}
+
+
 void convert_attrs(struct index_state *istate,
 		   struct conv_attrs *ca, const char *path)
 {
 	struct attr_check_item *ccheck = NULL;
+  struct strbuf platform_working_tree_encoding = STRBUF_INIT;
+
+	strbuf_addf(&platform_working_tree_encoding, "%s-working-tree-encoding", get_platform());
+
 
 	if (!check) {
 		check = attr_check_initl("crlf", "ident", "filter",
-					 "eol", "text", "working-tree-encoding",
+					 "eol", "text", "working-tree-encoding", platform_working_tree_encoding.buf,
 					 NULL);
 		user_convert_tail = &user_convert;
 		git_config(read_convert_config, NULL);
 	}
+	strbuf_release(&platform_working_tree_encoding);
 
 	git_check_attr(istate, path, check);
 	ccheck = check->items;
@@ -1334,6 +1364,8 @@ void convert_attrs(struct index_state *istate,
 			ca->crlf_action = CRLF_TEXT_CRLF;
 	}
 	ca->working_tree_encoding = git_path_check_encoding(ccheck + 5);
+  if (git_path_check_encoding(ccheck + 6))
+    ca->working_tree_encoding = git_path_check_encoding(ccheck + 6);
 
 	/* Save attr and make a decision for action */
 	ca->attr_action = ca->crlf_action;
@@ -1427,7 +1459,7 @@ int convert_to_git(struct index_state *istate,
 		len = dst->len;
 	}
 
-	ret |= encode_to_git(path, src, len, dst, ca.working_tree_encoding, conv_flags);
+	ret |= encode_to_git(path, src, len, dst, ca.working_tree_encoding, ca.attr_action, conv_flags);
 	if (ret && dst) {
 		src = dst->buf;
 		len = dst->len;
@@ -1455,7 +1487,7 @@ void convert_to_git_filter_fd(struct index_state *istate,
 	if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN, NULL, NULL))
 		die(_("%s: clean filter '%s' failed"), path, ca.drv->name);
 
-	encode_to_git(path, dst->buf, dst->len, dst, ca.working_tree_encoding, conv_flags);
+	encode_to_git(path, dst->buf, dst->len, dst, ca.working_tree_encoding, ca.attr_action, conv_flags);
 	crlf_to_git(istate, path, dst->buf, dst->len, dst, ca.crlf_action, conv_flags);
 	ident_to_git(dst->buf, dst->len, dst, ca.ident);
 }
@@ -1487,7 +1519,7 @@ static int convert_to_working_tree_ca_internal(const struct conv_attrs *ca,
 		}
 	}
 
-	ret |= encode_to_worktree(path, src, len, dst, ca->working_tree_encoding);
+	ret |= encode_to_worktree(path, src, len, dst, ca->attr_action, ca->working_tree_encoding);
 	if (ret) {
 		src = dst->buf;
 		len = dst->len;
diff --git a/copy.c b/copy.c
index 23d84c6c1db..63546aecf81 100644
--- a/copy.c
+++ b/copy.c
@@ -14,6 +14,9 @@ int copy_fd(int ifd, int ofd)
 		if (write_in_full(ofd, buffer, len) < 0)
 			return COPY_WRITE_ERROR;
 	}
+#ifdef __MVS__
+  __copyfdccsid(ifd, ofd);
+#endif
 	return 0;
 }
 
diff --git a/diff.c b/diff.c
index 2c602df10a3..28b96d53dbc 100644
--- a/diff.c
+++ b/diff.c
@@ -4083,6 +4083,9 @@ int diff_populate_filespec(struct repository *r,
 	int check_binary = options ? options->check_binary : 0;
 	int err = 0;
 	int conv_flags = global_conv_flags_eol;
+#ifdef __MVS__
+	int autocvtToASCII;
+#endif
 	/*
 	 * demote FAIL to WARN to allow inspecting the situation
 	 * instead of refusing.
@@ -4155,9 +4158,17 @@ int diff_populate_filespec(struct repository *r,
 			s->is_binary = 1;
 			return 0;
 		}
+#ifdef __MVS__
+    validate_codeset(r->index, s->path, &autocvtToASCII);
+#endif
 		fd = open(s->path, O_RDONLY);
 		if (fd < 0)
 			goto err_empty;
+
+#ifdef __MVS__
+    if (!autocvtToASCII)
+      __disableautocvt(fd);
+#endif
 		s->data = xmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);
 		close(fd);
 		s->should_munmap = 1;
diff --git a/entry.c b/entry.c
index 076e97eb89c..df6feb2234b 100644
--- a/entry.c
+++ b/entry.c
@@ -126,6 +126,24 @@ int fstat_checkout_output(int fd, const struct checkout *state, struct stat *st)
 	return 0;
 }
 
+#ifdef __MVS__
+void tag_file_as_working_tree_encoding(struct index_state *istate, char* path, int fd) {
+	struct conv_attrs ca;
+	convert_attrs(istate, &ca, path);
+  if (ca.attr_action != CRLF_BINARY) {
+    if (ca.working_tree_encoding)
+      __chgfdcodeset(fd, ca.working_tree_encoding);
+    else
+      __setfdtext(fd);
+  }
+  else {
+    __setfdbinary(fd);
+  }
+
+  __disableautocvt(fd);
+}
+#endif
+
 static int streaming_write_entry(const struct cache_entry *ce, char *path,
 				 struct stream_filter *filter,
 				 const struct checkout *state, int to_tempfile,
@@ -138,6 +156,10 @@ static int streaming_write_entry(const struct cache_entry *ce, char *path,
 	if (fd < 0)
 		return -1;
 
+#ifdef __MVS__
+  tag_file_as_working_tree_encoding(state->istate, path, fd);
+#endif
+
 	result |= stream_blob_to_fd(fd, &ce->oid, filter, 1);
 	*fstat_done = fstat_checkout_output(fd, state, statbuf);
 	result |= close(fd);
@@ -374,6 +396,10 @@ static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca
 			return error_errno("unable to create file %s", path);
 		}
 
+#ifdef __MVS__
+    tag_file_as_working_tree_encoding(state->istate, path, fd);
+#endif
+
 		wrote = write_in_full(fd, new_blob, size);
 		if (!to_tempfile)
 			fstat_done = fstat_checkout_output(fd, state, &st);
diff --git a/environment.c b/environment.c
index bb3c2a96a33..2e4d3a1e058 100644
--- a/environment.c
+++ b/environment.c
@@ -51,6 +51,9 @@ const char *git_hooks_path;
 int zlib_compression_level = Z_BEST_SPEED;
 int pack_compression_level = Z_DEFAULT_COMPRESSION;
 int fsync_object_files = -1;
+#ifdef __MVS__
+int ignore_file_tags = 0;
+#endif
 int use_fsync = -1;
 enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
 enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
diff --git a/git-compat-util.h b/git-compat-util.h
index 3e7a59b5ff1..66e0abec24b 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -223,7 +223,15 @@ struct strbuf;
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <stddef.h>
+#ifdef __MVS__
+#define release stdlib_release
+#define fetch stdlib_fetch
+#endif
 #include <stdlib.h>
+#ifdef __MVS__
+#undef fetch
+#undef release
+#endif
 #include <stdarg.h>
 #include <string.h>
 #ifdef HAVE_STRINGS_H
diff --git a/negotiator/default.c b/negotiator/default.c
index 9a5b6963272..b1f9f153372 100644
--- a/negotiator/default.c
+++ b/negotiator/default.c
@@ -174,7 +174,7 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
 	return known_to_be_common;
 }
 
-static void release(struct fetch_negotiator *n)
+static void release_negotiator(struct fetch_negotiator *n)
 {
 	clear_prio_queue(&((struct negotiation_state *)n->data)->rev_list);
 	FREE_AND_NULL(n->data);
@@ -187,7 +187,7 @@ void default_negotiator_init(struct fetch_negotiator *negotiator)
 	negotiator->add_tip = add_tip;
 	negotiator->next = next;
 	negotiator->ack = ack;
-	negotiator->release = release;
+	negotiator->release_negotiator = release_negotiator;
 	negotiator->data = CALLOC_ARRAY(ns, 1);
 	ns->rev_list.compare = compare_commits_by_commit_date;
 
diff --git a/negotiator/noop.c b/negotiator/noop.c
index de39028ab7f..82089654d8b 100644
--- a/negotiator/noop.c
+++ b/negotiator/noop.c
@@ -30,7 +30,7 @@ static int ack(struct fetch_negotiator *n UNUSED, struct commit *c UNUSED)
 	return 0;
 }
 
-static void release(struct fetch_negotiator *n UNUSED)
+static void release_negotiator(struct fetch_negotiator *n UNUSED)
 {
 	/* nothing to release */
 }
@@ -41,6 +41,6 @@ void noop_negotiator_init(struct fetch_negotiator *negotiator)
 	negotiator->add_tip = add_tip;
 	negotiator->next = next;
 	negotiator->ack = ack;
-	negotiator->release = release;
+	negotiator->release_negotiator = release_negotiator;
 	negotiator->data = NULL;
 }
diff --git a/negotiator/skipping.c b/negotiator/skipping.c
index 5b91520430c..783b3f27e63 100644
--- a/negotiator/skipping.c
+++ b/negotiator/skipping.c
@@ -243,7 +243,7 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
 	return known_to_be_common;
 }
 
-static void release(struct fetch_negotiator *n)
+static void release_negotiator(struct fetch_negotiator *n)
 {
 	clear_prio_queue(&((struct data *)n->data)->rev_list);
 	FREE_AND_NULL(n->data);
@@ -256,7 +256,7 @@ void skipping_negotiator_init(struct fetch_negotiator *negotiator)
 	negotiator->add_tip = add_tip;
 	negotiator->next = next;
 	negotiator->ack = ack;
-	negotiator->release = release;
+	negotiator->release_negotiator = release_negotiator;
 	negotiator->data = CALLOC_ARRAY(data, 1);
 	data->rev_list.compare = compare;
 
diff --git a/object-file.c b/object-file.c
index 7c7afe57936..28e69ed1e33 100644
--- a/object-file.c
+++ b/object-file.c
@@ -43,7 +43,9 @@
 #include "setup.h"
 #include "submodule.h"
 #include "fsck.h"
-
+#ifdef __MVS__
+#include <_Ccsid.h>
+#endif
 /* The maximum size for an object header. */
 #define MAX_HEADER_LEN 32
 
@@ -2478,6 +2480,68 @@ int index_fd(struct index_state *istate, struct object_id *oid,
 	return ret;
 }
 
+#ifdef __MVS__
+void validate_codeset(struct index_state *istate, const char *path, int* autoconvertToASCII) {
+       struct conv_attrs ca;
+  struct stat st;
+  unsigned short attr_ccsid;
+  unsigned short file_ccsid;
+
+  if (ignore_file_tags)
+   return;
+
+  *autoconvertToASCII = 0;
+       convert_attrs(istate, &ca, path);
+  if (ca.attr_action == CRLF_BINARY) {
+    attr_ccsid = FT_BINARY;
+  }
+  else if (ca.working_tree_encoding) {
+    attr_ccsid = __toCcsid(ca.working_tree_encoding);
+  }
+  else
+    attr_ccsid = 819;
+
+  if (stat(path, &st) < 0)
+    return;
+
+  file_ccsid = st.st_tag.ft_ccsid;
+
+  if (file_ccsid == FT_UNTAGGED) {
+    die("File %s is untagged, set the correct file tag (using the chtag command).", path);
+  }
+
+  if (attr_ccsid != file_ccsid) {
+    if (file_ccsid == 1047 && attr_ccsid == 819) {
+      *autoconvertToASCII = 1;
+      return;
+    }
+    // Allow tag mixing of 819 and 1208
+    if ((file_ccsid == 819 || file_ccsid == 1208) && (attr_ccsid == 1208 || attr_ccsid == 819)) {
+      return;
+    }
+    // Don't check for binary files, just add them
+    if (attr_ccsid == FT_BINARY)
+      return;
+
+    char attr_csname[_XOPEN_PATH_MAX] = {0};
+    char file_csname[_XOPEN_PATH_MAX] = {0};
+    if (attr_ccsid != FT_BINARY) {
+      __toCSName(attr_ccsid, attr_csname);
+    } else {
+      snprintf(attr_csname, _XOPEN_PATH_MAX, "%s", "binary");
+    }
+    if (file_ccsid != FT_BINARY) {
+      __toCSName(file_ccsid, file_csname);
+    } else {
+      snprintf(file_csname, _XOPEN_PATH_MAX, "%s", "binary");
+    }
+    die("%s added file: file tag (%s) does not match working-tree-encoding (%s)", path, file_csname, attr_csname);
+  }
+}
+#endif
+
+
+
 int index_path(struct index_state *istate, struct object_id *oid,
 	       const char *path, struct stat *st, unsigned flags)
 {
@@ -2485,11 +2549,25 @@ int index_path(struct index_state *istate, struct object_id *oid,
 	struct strbuf sb = STRBUF_INIT;
 	int rc = 0;
 
+#ifdef __MVS__
+	struct conv_attrs ca;
+	int autocvtToASCII;
+#endif
+
 	switch (st->st_mode & S_IFMT) {
 	case S_IFREG:
+#ifdef __MVS__
+    validate_codeset(istate, path, &autocvtToASCII);
+#endif
 		fd = open(path, O_RDONLY);
 		if (fd < 0)
 			return error_errno("open(\"%s\")", path);
+
+#ifdef __MVS__
+   if (!autocvtToASCII)
+     __disableautocvt(fd);
+#endif
+
 		if (index_fd(istate, oid, fd, st, OBJ_BLOB, path, flags) < 0)
 			return error(_("%s: failed to insert into database"),
 				     path);
diff --git a/read-cache.c b/read-cache.c
index 080bd39713b..75c06121302 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -205,6 +205,9 @@ static int ce_compare_data(struct index_state *istate,
 	int fd = git_open_cloexec(ce->name, O_RDONLY);
 
 	if (fd >= 0) {
+#ifdef __MVS__
+    __disableautocvt(fd);
+#endif
 		struct object_id oid;
 		if (!index_fd(istate, &oid, fd, st, OBJ_BLOB, ce->name, 0))
 			match = !oideq(&oid, &ce->oid);
diff --git a/utf8.c b/utf8.c
index 6a0dd25b0fe..b9cb56abf14 100644
--- a/utf8.c
+++ b/utf8.c
@@ -590,6 +590,17 @@ char *reencode_string_len(const char *in, size_t insz,
 #endif
 	}
 
+#ifdef __MVS__
+  //HACK: For backwards compat, ISO8859-1 really means utf-8 in the z/OS world
+  if (strcasecmp("ISO8859-1", in_encoding) == 0) {
+    in_encoding = "UTF-8";
+    out_encoding = "UTF-8";
+  }
+  if (strcasecmp("ISO8859-1", out_encoding) == 0) {
+    in_encoding = "UTF-8";
+    out_encoding = "UTF-8";
+  }
+#endif
 	conv = iconv_open(out_encoding, in_encoding);
 	if (conv == (iconv_t) -1) {
 		in_encoding = fallback_encoding(in_encoding);
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 03/13] spaces and errors fix Handled git pipeline errors
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
  2023-11-13 10:24 ` [PATCH 01/13] " Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 02/13] Enable builds for z/OS Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-14  0:38   ` Junio C Hamano
  2023-11-13 10:24 ` [PATCH 04/13] fixes for build errors Handled git pipeline errorse Haritha D via GitGitGadget
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This PR has fixes to enable build on z/OS

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 builtin/hash-object.c |  6 +++---
 convert.c             | 22 +++++++++++++++++-----
 entry.c               | 22 +++++++++++-----------
 object-file.c         |  6 +++---
 4 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index b33b32ff977..9129658a37c 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -62,8 +62,8 @@ static void hash_fd(int fd, const char *type, const char *path, unsigned flags,
 #  include <stdio.h>
 #  include <stdlib.h>
 
-   int setbinaryfd(int fd)
-   {
+int setbinaryfd(int fd)
+{
      attrib_t attr;
      int rc;
 
@@ -74,7 +74,7 @@ static void hash_fd(int fd, const char *type, const char *path, unsigned flags,
 
      rc = __fchattr(fd, &attr, sizeof(attr));
      return rc;
-   }
+}
 #  endif
 #endif
 
diff --git a/convert.c b/convert.c
index 4f14ff6f1ed..17cc849efed 100644
--- a/convert.c
+++ b/convert.c
@@ -1315,15 +1315,28 @@ static struct attr_check *check;
 
 static const char* get_platform() {
 	struct utsname uname_info;
+	char *result;
+	if(!uname_info.sysname)
+	{
+		result = (char *)malloc(strlen(uname_info.sysname)+1);
+		int index=0;
+		while(index <= strlen(uname_info.sysname))
+		{
+			*result = uname_info.sysname[index];
+			++result;
+			++index;
+		}
+	}
 
 	if (uname(&uname_info))
 		die(_("uname() failed with error '%s' (%d)\n"),
 			    strerror(errno),
 			    errno);
 
-  if (!strcmp(uname_info.sysname, "OS/390"))
-    return "zos";
-  return uname_info.sysname;
+	if (!strcmp(uname_info.sysname, "OS/390"))
+		result="zos";
+
+	return result;
 }
 
 
@@ -1331,11 +1344,10 @@ void convert_attrs(struct index_state *istate,
 		   struct conv_attrs *ca, const char *path)
 {
 	struct attr_check_item *ccheck = NULL;
-  struct strbuf platform_working_tree_encoding = STRBUF_INIT;
+	struct strbuf platform_working_tree_encoding = STRBUF_INIT;
 
 	strbuf_addf(&platform_working_tree_encoding, "%s-working-tree-encoding", get_platform());
 
-
 	if (!check) {
 		check = attr_check_initl("crlf", "ident", "filter",
 					 "eol", "text", "working-tree-encoding", platform_working_tree_encoding.buf,
diff --git a/entry.c b/entry.c
index df6feb2234b..f2a7b2adbf5 100644
--- a/entry.c
+++ b/entry.c
@@ -130,17 +130,17 @@ int fstat_checkout_output(int fd, const struct checkout *state, struct stat *st)
 void tag_file_as_working_tree_encoding(struct index_state *istate, char* path, int fd) {
 	struct conv_attrs ca;
 	convert_attrs(istate, &ca, path);
-  if (ca.attr_action != CRLF_BINARY) {
-    if (ca.working_tree_encoding)
-      __chgfdcodeset(fd, ca.working_tree_encoding);
-    else
-      __setfdtext(fd);
-  }
-  else {
-    __setfdbinary(fd);
-  }
-
-  __disableautocvt(fd);
+	if (ca.attr_action != CRLF_BINARY) {
+		if (ca.working_tree_encoding)
+			__chgfdcodeset(fd, ca.working_tree_encoding);
+		else
+			__setfdtext(fd);
+	}
+	else {
+		__setfdbinary(fd);
+	}
+
+	__disableautocvt(fd);
 }
 #endif
 
diff --git a/object-file.c b/object-file.c
index 28e69ed1e33..562d1344422 100644
--- a/object-file.c
+++ b/object-file.c
@@ -2557,15 +2557,15 @@ int index_path(struct index_state *istate, struct object_id *oid,
 	switch (st->st_mode & S_IFMT) {
 	case S_IFREG:
 #ifdef __MVS__
-    validate_codeset(istate, path, &autocvtToASCII);
+	validate_codeset(istate, path, &autocvtToASCII);
 #endif
 		fd = open(path, O_RDONLY);
 		if (fd < 0)
 			return error_errno("open(\"%s\")", path);
 
 #ifdef __MVS__
-   if (!autocvtToASCII)
-     __disableautocvt(fd);
+	if (!autocvtToASCII)
+		__disableautocvt(fd);
 #endif
 
 		if (index_fd(istate, oid, fd, st, OBJ_BLOB, path, flags) < 0)
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 04/13] fixes for build errors Handled git pipeline errorse
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (2 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 03/13] spaces and errors fix Handled git pipeline errors Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 05/13] fixes for build errors Haritha D via GitGitGadget
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This PR has fixes to enable build on z/OS

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 convert.c          | 4 ++--
 fetch-negotiator.h | 2 +-
 fetch-pack.c       | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/convert.c b/convert.c
index 17cc849efed..da05f6c2e51 100644
--- a/convert.c
+++ b/convert.c
@@ -1313,13 +1313,13 @@ static int git_path_check_ident(struct attr_check_item *check)
 
 static struct attr_check *check;
 
-static const char* get_platform() {
+static const char* get_platform(void) {
 	struct utsname uname_info;
 	char *result;
 	if(!uname_info.sysname)
 	{
-		result = (char *)malloc(strlen(uname_info.sysname)+1);
 		int index=0;
+		result = (char *)malloc(strlen(uname_info.sysname)+1);
 		while(index <= strlen(uname_info.sysname))
 		{
 			*result = uname_info.sysname[index];
diff --git a/fetch-negotiator.h b/fetch-negotiator.h
index e348905a1f0..2e19ef247f9 100644
--- a/fetch-negotiator.h
+++ b/fetch-negotiator.h
@@ -47,7 +47,7 @@ struct fetch_negotiator {
 	 */
 	int (*ack)(struct fetch_negotiator *, struct commit *);
 
-	void (*release)(struct fetch_negotiator *);
+	void (*release_negotiator)(struct fetch_negotiator *);
 
 	/* internal use */
 	void *data;
diff --git a/fetch-pack.c b/fetch-pack.c
index 26999e3b659..f40b90dfa65 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1232,7 +1232,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 
  all_done:
 	if (negotiator)
-		negotiator->release(negotiator);
+		negotiator->release_negotiator(negotiator);
 	return ref;
 }
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 05/13] fixes for build errors
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (3 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 04/13] fixes for build errors Handled git pipeline errorse Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 06/13] spaces and errors fix Handled git pipeline errors Haritha D via GitGitGadget
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This patch has fixes for build errors

Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
 convert.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/convert.c b/convert.c
index da05f6c2e51..d3f204b4c29 100644
--- a/convert.c
+++ b/convert.c
@@ -1315,8 +1315,8 @@ static struct attr_check *check;
 
 static const char* get_platform(void) {
 	struct utsname uname_info;
-	char *result;
-	if(!uname_info.sysname)
+	char *result = NULL;
+	if(!uname_info.sysname[0])
 	{
 		int index=0;
 		result = (char *)malloc(strlen(uname_info.sysname)+1);
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 06/13] spaces and errors fix Handled git pipeline errors
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (4 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 05/13] fixes for build errors Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 07/13] " Haritha D via GitGitGadget
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This PR has fixes to enable build on z/OS

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 convert.c    | 2 +-
 fetch-pack.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/convert.c b/convert.c
index d3f204b4c29..7fe107710ec 100644
--- a/convert.c
+++ b/convert.c
@@ -1314,7 +1314,7 @@ static int git_path_check_ident(struct attr_check_item *check)
 static struct attr_check *check;
 
 static const char* get_platform(void) {
-	struct utsname uname_info;
+	struct utsname uname_info = {0};
 	char *result = NULL;
 	if(!uname_info.sysname[0])
 	{
diff --git a/fetch-pack.c b/fetch-pack.c
index f40b90dfa65..c1f2e714f8e 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1853,7 +1853,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 		die("fsck failed");
 
 	if (negotiator)
-		negotiator->release(negotiator);
+		negotiator->release_negotiator(negotiator);
 
 	oidset_clear(&common);
 	return ref;
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 07/13] spaces and errors fix Handled git pipeline errors
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (5 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 06/13] spaces and errors fix Handled git pipeline errors Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 08/13] platform_name " Haritha D via GitGitGadget
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This PR has fixes to enable build on z/OS

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 convert.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/convert.c b/convert.c
index 7fe107710ec..c8d30011458 100644
--- a/convert.c
+++ b/convert.c
@@ -1314,9 +1314,15 @@ static int git_path_check_ident(struct attr_check_item *check)
 static struct attr_check *check;
 
 static const char* get_platform(void) {
-	struct utsname uname_info = {0};
+	struct utsname uname_info;
 	char *result = NULL;
-	if(!uname_info.sysname[0])
+
+	if (uname(&uname_info) < 0)
+		die(_("uname() failed with error '%s' (%d)\n"),
+			strerror(errno),
+			errno);
+
+	if(*uname_info.sysname != '\0')
 	{
 		int index=0;
 		result = (char *)malloc(strlen(uname_info.sysname)+1);
@@ -1326,15 +1332,13 @@ static const char* get_platform(void) {
 			++result;
 			++index;
 		}
+		*result = '\0';
 	}
 
-	if (uname(&uname_info))
-		die(_("uname() failed with error '%s' (%d)\n"),
-			    strerror(errno),
-			    errno);
-
+#ifdef __MVS__
 	if (!strcmp(uname_info.sysname, "OS/390"))
 		result="zos";
+#endif
 
 	return result;
 }
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 08/13] platform_name fix Handled git pipeline errors
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (6 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 07/13] " Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 09/13] strncpy " Haritha D via GitGitGadget
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This PR has fixes to enable build on z/OS

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 convert.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/convert.c b/convert.c
index c8d30011458..9cd0c1382ac 100644
--- a/convert.c
+++ b/convert.c
@@ -1321,19 +1321,14 @@ static const char* get_platform(void) {
 		die(_("uname() failed with error '%s' (%d)\n"),
 			strerror(errno),
 			errno);
-
+#ifndef __MVS__
 	if(*uname_info.sysname != '\0')
 	{
 		int index=0;
 		result = (char *)malloc(strlen(uname_info.sysname)+1);
-		while(index <= strlen(uname_info.sysname))
-		{
-			*result = uname_info.sysname[index];
-			++result;
-			++index;
-		}
-		*result = '\0';
+		strncpy(result, uname_info.sysname, strlen(uname_info.sysname));
 	}
+#endif
 
 #ifdef __MVS__
 	if (!strcmp(uname_info.sysname, "OS/390"))
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 09/13] strncpy fix Handled git pipeline errors
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (7 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 08/13] platform_name " Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 10/13] " Haritha D via GitGitGadget
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This PR has fixes to enable build on z/OS

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 convert.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/convert.c b/convert.c
index 9cd0c1382ac..78403de422d 100644
--- a/convert.c
+++ b/convert.c
@@ -1326,7 +1326,13 @@ static const char* get_platform(void) {
 	{
 		int index=0;
 		result = (char *)malloc(strlen(uname_info.sysname)+1);
-		strncpy(result, uname_info.sysname, strlen(uname_info.sysname));
+		while(index <= strlen(uname_info.sysname))
+		{
+			*result = uname_info.sysname[index];
+			++result;
+			++index;
+		}
+		*result = '\0';
 	}
 #endif
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 10/13] strncpy fix Handled git pipeline errors
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (8 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 09/13] strncpy " Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 11/13] " Haritha D via GitGitGadget
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This PR has fixes to enable build on z/OS

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 convert.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/convert.c b/convert.c
index 78403de422d..ef44e6429da 100644
--- a/convert.c
+++ b/convert.c
@@ -1326,13 +1326,10 @@ static const char* get_platform(void) {
 	{
 		int index=0;
 		result = (char *)malloc(strlen(uname_info.sysname)+1);
-		while(index <= strlen(uname_info.sysname))
+		while(result[index] = uname_info.sysname[index])
 		{
-			*result = uname_info.sysname[index];
-			++result;
-			++index;
+			index++;
 		}
-		*result = '\0';
 	}
 #endif
 
@@ -1340,8 +1337,7 @@ static const char* get_platform(void) {
 	if (!strcmp(uname_info.sysname, "OS/390"))
 		result="zos";
 #endif
-
-	return result;
+	return (char*)result;
 }
 
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 11/13] strncpy fix Handled git pipeline errors
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (9 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 10/13] " Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 12/13] Handled git pipeline errors - Memory leak Haritha D via GitGitGadget
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This PR has fixes to enable build on z/OS

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 convert.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/convert.c b/convert.c
index ef44e6429da..16173a1caf6 100644
--- a/convert.c
+++ b/convert.c
@@ -1326,8 +1326,9 @@ static const char* get_platform(void) {
 	{
 		int index=0;
 		result = (char *)malloc(strlen(uname_info.sysname)+1);
-		while(result[index] = uname_info.sysname[index])
+		while(index <= strlen(uname_info.sysname))
 		{
+			result[index] = uname_info.sysname[index];
 			index++;
 		}
 	}
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 12/13] Handled git pipeline errors - Memory leak
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (10 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 11/13] " Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-11-13 10:24 ` [PATCH 13/13] Handled git pipeline errors - z/OS enable Haritha D via GitGitGadget
  2023-12-04 14:19 ` [PATCH v2] This PR enables a successful git build on z/OS Haritha  via GitGitGadget
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This PR has fixes to enable build on z/OS

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 convert.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/convert.c b/convert.c
index 16173a1caf6..4c034bb714c 100644
--- a/convert.c
+++ b/convert.c
@@ -1313,9 +1313,9 @@ static int git_path_check_ident(struct attr_check_item *check)
 
 static struct attr_check *check;
 
-static const char* get_platform(void) {
+static void get_platform(char** result) {
 	struct utsname uname_info;
-	char *result = NULL;
+	*result = NULL;
 
 	if (uname(&uname_info) < 0)
 		die(_("uname() failed with error '%s' (%d)\n"),
@@ -1325,10 +1325,10 @@ static const char* get_platform(void) {
 	if(*uname_info.sysname != '\0')
 	{
 		int index=0;
-		result = (char *)malloc(strlen(uname_info.sysname)+1);
+		*result = (char *)malloc(strlen(uname_info.sysname)+1);
 		while(index <= strlen(uname_info.sysname))
 		{
-			result[index] = uname_info.sysname[index];
+			(*result)[index] = uname_info.sysname[index];
 			index++;
 		}
 	}
@@ -1336,9 +1336,8 @@ static const char* get_platform(void) {
 
 #ifdef __MVS__
 	if (!strcmp(uname_info.sysname, "OS/390"))
-		result="zos";
+		*result = "zos";
 #endif
-	return (char*)result;
 }
 
 
@@ -1347,8 +1346,11 @@ void convert_attrs(struct index_state *istate,
 {
 	struct attr_check_item *ccheck = NULL;
 	struct strbuf platform_working_tree_encoding = STRBUF_INIT;
-
-	strbuf_addf(&platform_working_tree_encoding, "%s-working-tree-encoding", get_platform());
+	char* result=NULL;
+	get_platform(&result);
+	strbuf_addf(&platform_working_tree_encoding, "%s-working-tree-encoding", result);
+	if (result != NULL)
+		free (result);
 
 	if (!check) {
 		check = attr_check_initl("crlf", "ident", "filter",
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 13/13] Handled git pipeline errors - z/OS enable
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (11 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 12/13] Handled git pipeline errors - Memory leak Haritha D via GitGitGadget
@ 2023-11-13 10:24 ` Haritha D via GitGitGadget
  2023-12-04 14:19 ` [PATCH v2] This PR enables a successful git build on z/OS Haritha  via GitGitGadget
  13 siblings, 0 replies; 21+ messages in thread
From: Haritha D via GitGitGadget @ 2023-11-13 10:24 UTC (permalink / raw)
  To: git; +Cc: Haritha, Haritha D

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

This PR has fixes to enable build on z/OS

Signed-off-by: Harithamma D <harithamma.d@ibm.com>
---
 convert.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/convert.c b/convert.c
index 4c034bb714c..c66f5b3ecec 100644
--- a/convert.c
+++ b/convert.c
@@ -1315,7 +1315,6 @@ static struct attr_check *check;
 
 static void get_platform(char** result) {
 	struct utsname uname_info;
-	*result = NULL;
 
 	if (uname(&uname_info) < 0)
 		die(_("uname() failed with error '%s' (%d)\n"),
@@ -1346,11 +1345,10 @@ void convert_attrs(struct index_state *istate,
 {
 	struct attr_check_item *ccheck = NULL;
 	struct strbuf platform_working_tree_encoding = STRBUF_INIT;
-	char* result=NULL;
+	char* result="Unknown";
 	get_platform(&result);
 	strbuf_addf(&platform_working_tree_encoding, "%s-working-tree-encoding", result);
-	if (result != NULL)
-		free (result);
+	free (result);
 
 	if (!check) {
 		check = attr_check_initl("crlf", "ident", "filter",
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/13] Enable builds for z/OS.
  2023-11-13 10:24 ` [PATCH 02/13] Enable builds for z/OS Haritha D via GitGitGadget
@ 2023-11-13 23:03   ` brian m. carlson
  2023-11-14  1:47     ` Junio C Hamano
  2023-11-14  0:48   ` Junio C Hamano
  1 sibling, 1 reply; 21+ messages in thread
From: brian m. carlson @ 2023-11-13 23:03 UTC (permalink / raw)
  To: Haritha D via GitGitGadget; +Cc: git, Haritha

[-- Attachment #1: Type: text/plain, Size: 10819 bytes --]

On 2023-11-13 at 10:24:04, Haritha D via GitGitGadget wrote:
> From: Haritha D <harithamma.d@ibm.com>
> 
> This commit enables git to build on z/OS.
> It takes advantage of enahanced ASCII
> services on z/OS to auto-convert input
> files to ASCII

In general, we don't want to convert files to ASCII, since we assume
text files are UTF-8 unless otherwise specified.  If you're suggesting
that your system is normally EBCDIC, changing that should be an initial
piece of setup code or something used in `xopen` on your platform so it
applies everywhere with minimal changes.

> It also adds support for
> [platform]-working-tree-encoding.
> Platform is substituted with uname_info.sysname,
> so it will only apply to the given platform.

I think this is going to need to be a separate patch and feature with
its own explanation of why it's valuable.

> Also adds support for scripts that are not in
> standard locations so that /bin/env bash
> can be specified.


> Signed-off-by: Harithamma D <harithamma.d@ibm.com>
> ---
>  Makefile              | 21 +++++++++---
>  builtin.h             |  3 ++
>  builtin/archive.c     |  6 ++++
>  builtin/hash-object.c | 28 +++++++++++++++
>  combine-diff.c        |  4 +++
>  config.c              |  7 ++++
>  configure.ac          |  3 ++
>  convert.c             | 44 ++++++++++++++++++++----
>  copy.c                |  3 ++
>  diff.c                | 11 ++++++
>  entry.c               | 26 ++++++++++++++
>  environment.c         |  3 ++
>  git-compat-util.h     |  8 +++++
>  negotiator/default.c  |  4 +--
>  negotiator/noop.c     |  4 +--
>  negotiator/skipping.c |  4 +--
>  object-file.c         | 80 ++++++++++++++++++++++++++++++++++++++++++-
>  read-cache.c          |  3 ++
>  utf8.c                | 11 ++++++
>  19 files changed, 255 insertions(+), 18 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9c6a2f125f8..30aa76da4f4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -20,6 +20,8 @@ include shared.mak
>  #
>  # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
>  #
> +# Define SHELL_PATH_FOR_SCRIPTS to a POSIX shell if your /bin/sh is broken.
> +#
>  # Define SANE_TOOL_PATH to a colon-separated list of paths to prepend
>  # to PATH if your tools in /usr/bin are broken.
>  #
> @@ -215,6 +217,8 @@ include shared.mak
>  #
>  # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl).
>  #
> +# Define PERL_PATH_FOR_SCRIPTS to a Perl binary if your /usr/bin/perl is broken.

You will probably want to explain in your commit message why these two
are required to be different from the standard values and explain here
what the relevant difference is so that users can set them
appropriately.

> diff --git a/builtin/archive.c b/builtin/archive.c
> index 90761fdfee0..53ec794356f 100644
> --- a/builtin/archive.c
> +++ b/builtin/archive.c
> @@ -14,6 +14,12 @@
>  static void create_output_file(const char *output_file)
>  {
>  	int output_fd = xopen(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
> +#ifdef __MVS__
> + #if (__CHARSET_LIB == 1)
> +	if (setbinaryfd(output_fd))
> +		die_errno(_("could not tag archive file '%s'"), output_file);
> + #endif
> +#endif

This would be better to place in `xopen` itself so that all files are
correctly configured.  That would do well as its own patch, and you'd
want to explain well in the commit message what this function does, why
it's necessary, and what the consequences of not using it are, as well
as any alternatives that you've rejected.

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

We typically don't include the standard headers here.  Instead, they're
included by git-compat-util.h because on some systems they have to be
included in a certain order with certain options.  If you include that
header instead at the top of the file, or one of the headers that
includes it, then typically that should do the right thing.

> +   int setbinaryfd(int fd)
> +   {
> +     attrib_t attr;
> +     int rc;
> +
> +     memset(&attr, 0, sizeof(attr));
> +     attr.att_filetagchg = 1;
> +     attr.att_filetag.ft_ccsid = FT_BINARY;
> +     attr.att_filetag.ft_txtflag = 0;
> +
> +     rc = __fchattr(fd, &attr, sizeof(attr));
> +     return rc;
> +   }
> +#  endif
> +#endif

I would think a comment explaining what this function does and why it's
necessary would be appropriate, since it's not in POSIX and isn't
typically necessary on POSIX systems.

> diff --git a/combine-diff.c b/combine-diff.c
> index f90f4424829..73445a517c7 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -1082,6 +1082,10 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
>  			ssize_t done;
>  			int is_file, i;
>  
> +#ifdef __MVS__
> +      __disableautocvt(fd);
> +#endif

Again, if this can be centralized, it should be, and it should be
explained well in the commit message.  I'm uncertain what it does or
what value it provides.

> diff --git a/config.c b/config.c
> index f9a1cca4e8a..37c124a37c0 100644
> --- a/config.c
> +++ b/config.c
> @@ -1521,6 +1521,13 @@ static int git_default_core_config(const char *var, const char *value,
>  		return 0;
>  	}
>  
> +	#ifdef __MVS__
> +	if (!strcmp(var, "core.ignorefiletags")) {
> +		ignore_file_tags = git_config_bool(var, value);
> +		return 0;
> +	}
> +	#endif

This should also live in its own patch and the commit message should
explain what it does.  We'd also want it to be documented in the config
options in the Documentation directory.

> diff --git a/convert.c b/convert.c
> index a8870baff36..4f14ff6f1ed 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -377,12 +377,15 @@ static int check_roundtrip(const char *enc_name)
>  static const char *default_encoding = "UTF-8";
>  
>  static int encode_to_git(const char *path, const char *src, size_t src_len,
> -			 struct strbuf *buf, const char *enc, int conv_flags)
> +			 struct strbuf *buf, const char *enc, enum convert_crlf_action attr_action, int conv_flags)
>  {
>  	char *dst;
>  	size_t dst_len;
>  	int die_on_error = conv_flags & CONV_WRITE_OBJECT;
>  
> +  if (attr_action == CRLF_BINARY) {
> +    return 0;
> +  }

I'm pretty sure this is a change in behaviour from what we had before.
It should live in its own patch, with an explanation in the commit
message why it's a compelling and correct change overall, and with
suitable tests.

>  	/*
>  	 * No encoding is specified or there is nothing to encode.
>  	 * Tell the caller that the content was not modified.
> @@ -403,6 +406,11 @@ static int encode_to_git(const char *path, const char *src, size_t src_len,
>  		return 0;
>  
>  	trace_encoding("source", path, enc, src, src_len);
> +#ifdef __MVS__
> +  // Don't convert ISO8859-1 on z/OS
> +  if (strcasecmp("ISO8859-1", enc) == 0)
> +    return 0;
> +#endif

This definitely needs explanation in the commit message and should
probably be its own patch, explaining why z/OS has this compelling need
to not convert ISO8859-1.  Note that ISO8859-1 is not the same as "no
binary conversion", since it doesn't include many control codes.

Note that if, as it says later on, this really means "UTF-8", that's a
platform wart you'd want to paper over in a file in the compat code.  In
general, the compat directory is a good place to put anything that your
platform needs specifically.

> +static const char* get_platform() {
> +	struct utsname uname_info;
> +
> +	if (uname(&uname_info))
> +		die(_("uname() failed with error '%s' (%d)\n"),
> +			    strerror(errno),
> +			    errno);
> +
> +  if (!strcmp(uname_info.sysname, "OS/390"))
> +    return "zos";
> +  return uname_info.sysname;
> +}

This is definitely a new feature, and I'm not sure why it's necessary or
useful.  I suspect there's something about z/OS that makes it valuable,
but I don't know what it is since the commit message doesn't tell me.
I'm also not sure that these values will be correct on Windows.

I think I could go on to make similar comments about the rest of this
series.  I'm not opposed to seeing z/OS changes come in, but you've
amalgamated at least a half-dozen separate patches into one and haven't
explained them very thoroughly in the commit message.

I'd generally want to look at the commit message and understand the
problem the code is trying to solve and then look at the code and think,
"Oh, yes, this seems like the obvious and logical way to solve this
problem," or at least think, "Oh, no, I think we should solve this
problem in a different way," so I can help make a thoughtful review
comment.  Right now, I lack the information to have an informed opinion
and so I can't provide any helpful feedback or analysis of the patches.

When you're adding new features or fixing bugs, we'll also want tests
for those cases to help us avoid regressing that code in the future.
Even if we don't normally run the testsuite on z/OS, at least _you_ will
notice that the tests have failed and then we'll be able to address the
bugs in a timely manner.

I also noted that there were some fixup commits later on in the series
that address whitespace issues.  Typically, we'd want to squash those in
to the earlier patches.  Nobody expects perfection, but squashing errors
into earlier patches helps us keep the history neat and let us pretend
like you never made those errors at all.  It also lets tools like git
blame and git bisect work more nicely for users.

I'd recommend a quick pass over the SubmittingPatches file, which is
also available at https://git-scm.com/docs/SubmittingPatches.  The
sections on making separate commits for separate changes and describing
changes well come to mind as places to focus.

I know this may seem overwhelming and like I'm upset or disappointed;
I'm definitely not.  I'm very much interested in seeing Git available
for more platforms, but right now it's too hard for me to reason about
the changes for z/OS to provide helpful feedback, so I'm hoping you can
send a fixed v2 that helps me (and everyone else) understand these
changes better so you can get a helpful review.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 03/13] spaces and errors fix Handled git pipeline errors
  2023-11-13 10:24 ` [PATCH 03/13] spaces and errors fix Handled git pipeline errors Haritha D via GitGitGadget
@ 2023-11-14  0:38   ` Junio C Hamano
  0 siblings, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2023-11-14  0:38 UTC (permalink / raw)
  To: Haritha D via GitGitGadget; +Cc: git, Haritha

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

> Subject: Re: [PATCH 03/13] spaces and errors fix Handled git pipeline errors

-ECANNOTPARSE.  Perhaps Documentation/CodingGuidelines and
 Documentation/SubmittingPatches may help?

> From: Haritha D <harithamma.d@ibm.com>
>
> This PR has fixes to enable build on z/OS

This is way under-explained.  Your proposed log message should be
able to answer when somebody asks "Is anything broken in the
existing codebase to cause your build to fail, or is it your
compiler toolchain that is broken?" but the above does not help
understanding what and why you needed to fix at all.

> diff --git a/builtin/hash-object.c b/builtin/hash-object.c
> index b33b32ff977..9129658a37c 100644
> --- a/builtin/hash-object.c
> +++ b/builtin/hash-object.c
> @@ -62,8 +62,8 @@ static void hash_fd(int fd, const char *type, const char *path, unsigned flags,
>  #  include <stdio.h>
>  #  include <stdlib.h>
>  
> -   int setbinaryfd(int fd)
> -   {
> +int setbinaryfd(int fd)
> +{
>       attrib_t attr;
>       int rc;
>  
> @@ -74,7 +74,7 @@ static void hash_fd(int fd, const char *type, const char *path, unsigned flags,
>  
>       rc = __fchattr(fd, &attr, sizeof(attr));
>       return rc;
> -   }
> +}
>  #  endif
>  #endif

No such function in our codebase.  Are you fixing somebody else's
forked version of Git and we shouldn't even be looking at this
patch, perhaps?

> diff --git a/convert.c b/convert.c
> index 4f14ff6f1ed..17cc849efed 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -1315,15 +1315,28 @@ static struct attr_check *check;
>  
>  static const char* get_platform() {
>  	struct utsname uname_info;
> +	char *result;
> +	if(!uname_info.sysname)
> +	{
> +		result = (char *)malloc(strlen(uname_info.sysname)+1);
> +		int index=0;
> +		while(index <= strlen(uname_info.sysname))
> +		{
> +			*result = uname_info.sysname[index];
> +			++result;
> +			++index;
> +		}
> +	}

No such function in our codebase.  I doubt these patches have much
relevance to this project?

I'll stop here.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/13] Enable builds for z/OS.
  2023-11-13 10:24 ` [PATCH 02/13] Enable builds for z/OS Haritha D via GitGitGadget
  2023-11-13 23:03   ` brian m. carlson
@ 2023-11-14  0:48   ` Junio C Hamano
  1 sibling, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2023-11-14  0:48 UTC (permalink / raw)
  To: Haritha D via GitGitGadget; +Cc: git, Haritha

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

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

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

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

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

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

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

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

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

I'll stop here.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/13] Enable builds for z/OS.
  2023-11-13 23:03   ` brian m. carlson
@ 2023-11-14  1:47     ` Junio C Hamano
  0 siblings, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2023-11-14  1:47 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Haritha D via GitGitGadget, git, Haritha

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> I'd generally want to look at the commit message and understand the
> problem the code is trying to solve and then look at the code and think,
> "Oh, yes, this seems like the obvious and logical way to solve this
> problem," or at least think, "Oh, no, I think we should solve this
> problem in a different way," so I can help make a thoughtful review
> comment.  Right now, I lack the information to have an informed opinion
> and so I can't provide any helpful feedback or analysis of the patches.
> ...
> I'd recommend a quick pass over the SubmittingPatches file, which is
> also available at https://git-scm.com/docs/SubmittingPatches.  The
> sections on making separate commits for separate changes and describing
> changes well come to mind as places to focus.
>
> I know this may seem overwhelming and like I'm upset or disappointed;
> I'm definitely not.  I'm very much interested in seeing Git available
> for more platforms, but right now it's too hard for me to reason about
> the changes for z/OS to provide helpful feedback, so I'm hoping you can
> send a fixed v2 that helps me (and everyone else) understand these
> changes better so you can get a helpful review.

All very good pieces of advice.  I suspect we are missing some of
them from our SubmittingPatches or CodingGuidelines documents and
may want to add them there.

Thanks.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2] This PR enables a successful git build on z/OS.
  2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
                   ` (12 preceding siblings ...)
  2023-11-13 10:24 ` [PATCH 13/13] Handled git pipeline errors - z/OS enable Haritha D via GitGitGadget
@ 2023-12-04 14:19 ` Haritha  via GitGitGadget
  2023-12-04 21:46   ` Eric Sunshine
  13 siblings, 1 reply; 21+ messages in thread
From: Haritha  via GitGitGadget @ 2023-12-04 14:19 UTC (permalink / raw)
  To: git; +Cc: brian m. carlson, Haritha, Haritha D

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

Rename functions like "release" and "fetch"
due to conflict in z/OS standard C libraries.
Also disables autoconversion facility on z/OS
and relies on iconv.
New files created in binary format are also
tagged as binary.

Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
    Enabling z/OS workflow for git
    
    z/OS is an IBM mainframe operating system, also known as OS/390. Our
    team has been actively involved in porting Git to z/OS and we have made
    significant modifications to facilitate this process. The patch below is
    the initial configuration for z/OS. I also have few follow up changes
    and I will send that after these changes are approved. Please let me
    know if there are any concerns.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1537%2FHarithaIBM%2Fenablezos-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1537/HarithaIBM/enablezos-v2
Pull-Request: https://github.com/git/git/pull/1537

Range-diff vs v1:

  1:  712eb3712f1 <  -:  ----------- Enabling z/OS workflow for git
  2:  098b9ca8ece !  1:  b9882d90c5d Enable builds for z/OS.
     @@ Metadata
      Author: Haritha D <harithamma.d@ibm.com>
      
       ## Commit message ##
     -    Enable builds for z/OS.
     +    This PR enables a successful git build on z/OS.
      
     -    This commit enables git to build on z/OS.
     -    It takes advantage of enahanced ASCII
     -    services on z/OS to auto-convert input
     -    files to ASCII
     -    It also adds support for
     -    [platform]-working-tree-encoding.
     -    Platform is substituted with uname_info.sysname,
     -    so it will only apply to the given platform.
     -    Also adds support for scripts that are not in
     -    standard locations so that /bin/env bash
     -    can be specified.
     +    Rename functions like "release" and "fetch"
     +    due to conflict in z/OS standard C libraries.
     +    Also disables autoconversion facility on z/OS
     +    and relies on iconv.
     +    New files created in binary format are also
     +    tagged as binary.
      
     -    Signed-off-by: Harithamma D <harithamma.d@ibm.com>
     -
     - ## Makefile ##
     -@@ Makefile: include shared.mak
     - #
     - # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
     - #
     -+# Define SHELL_PATH_FOR_SCRIPTS to a POSIX shell if your /bin/sh is broken.
     -+#
     - # Define SANE_TOOL_PATH to a colon-separated list of paths to prepend
     - # to PATH if your tools in /usr/bin are broken.
     - #
     -@@ Makefile: include shared.mak
     - #
     - # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl).
     - #
     -+# Define PERL_PATH_FOR_SCRIPTS to a Perl binary if your /usr/bin/perl is broken.
     -+#
     - # Define NO_PERL if you do not want Perl scripts or libraries at all.
     - #
     - # Define NO_PERL_CPAN_FALLBACKS if you do not want to install bundled
     -@@ Makefile: BINDIR_PROGRAMS_NO_X += git-cvsserver
     - ifndef SHELL_PATH
     - 	SHELL_PATH = /bin/sh
     - endif
     -+ifndef SHELL_PATH_FOR_SCRIPTS
     -+	SHELL_PATH_FOR_SCRIPTS = /bin/sh
     -+endif
     - ifndef PERL_PATH
     - 	PERL_PATH = /usr/bin/perl
     - endif
     -+ifndef PERL_PATH_FOR_SCRIPTS
     -+	PERL_PATH_FOR_SCRIPTS = /usr/bin/perl
     -+endif
     - ifndef PYTHON_PATH
     - 	PYTHON_PATH = /usr/bin/python
     - endif
     -@@ Makefile: THIRD_PARTY_SOURCES += sha1dc/%
     - 
     - # xdiff and reftable libs may in turn depend on what is in libgit.a
     - GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE)
     --EXTLIBS =
     -+EXTLIBS = $(ZOPEN_EXTRA_LIBS)
     - 
     - GIT_USER_AGENT = git/$(GIT_VERSION)
     - 
     -@@ Makefile: perllibdir_relative_SQ = $(subst ','\'',$(perllibdir_relative))
     - gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
     - gitwebstaticdir_SQ = $(subst ','\'',$(gitwebstaticdir))
     - 
     --SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
     -+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH_FOR_SCRIPTS))
     - TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
     - PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
     -+PERL_PATH_FOR_SCRIPTS_SQ = $(subst ','\'',$(PERL_PATH_FOR_SCRIPTS))
     - PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
     - TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
     - DIFF_SQ = $(subst ','\'',$(DIFF))
     -@@ Makefile: hook-list.h: generate-hooklist.sh Documentation/githooks.txt
     - 
     - SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
     - 	$(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
     --	$(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\
     -+	$(gitwebdir_SQ):$(PERL_PATH_FOR_SCRIPTS_SQ):$(PAGER_ENV):\
     - 	$(perllibdir_SQ)
     - GIT-SCRIPT-DEFINES: FORCE
     - 	@FLAGS='$(SCRIPT_DEFINES)'; \
     -@@ Makefile: sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
     -     -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \
     -     -e $(BROKEN_PATH_FIX) \
     -     -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \
     --    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
     -+    -e 's|@@PERL@@|$(PERL_PATH_FOR_SCRIPTS_SQ)|g' \
     -     -e 's|@@PAGER_ENV@@|$(PAGER_ENV_SQ)|g' \
     -     $@.sh >$@+
     - endef
     -@@ Makefile: PERL_DEFINES += $(gitexecdir) $(perllibdir) $(localedir)
     - $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
     - 	$(QUIET_GEN) \
     - 	sed -e '1{' \
     --	    -e '	s|#!.*perl|#!$(PERL_PATH_SQ)|' \
     -+	    -e '	s|#!.*perl|#!$(PERL_PATH_FOR_SCRIPTS_SQ)|' \
     - 	    -e '	r GIT-PERL-HEADER' \
     - 	    -e '	G' \
     - 	    -e '}' \
     -
     - ## builtin.h ##
     -@@ builtin.h: int cmd_verify_pack(int argc, const char **argv, const char *prefix);
     - int cmd_show_ref(int argc, const char **argv, const char *prefix);
     - int cmd_pack_refs(int argc, const char **argv, const char *prefix);
     - int cmd_replace(int argc, const char **argv, const char *prefix);
     -+#ifdef __MVS__
     -+  extern int setbinaryfd(int);
     -+#endif
     - 
     - #endif
     +    Signed-off-by: Haritha D <harithamma.d@ibm.com>
      
       ## builtin/archive.c ##
      @@
     @@ builtin/archive.c
       {
       	int output_fd = xopen(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
      +#ifdef __MVS__
     -+ #if (__CHARSET_LIB == 1)
     -+	if (setbinaryfd(output_fd))
     ++	/*
     ++	 * Since the data is in binary format,
     ++	 * we need to set the z/OS file tag
     ++	 * to binary to disable autoconversion
     ++	 */
     ++	if (__setfdbinary(output_fd))
      +		die_errno(_("could not tag archive file '%s'"), output_file);
     -+ #endif
      +#endif
       	if (output_fd != 1) {
       		if (dup2(output_fd, 1) < 0)
       			die_errno(_("could not redirect output"));
      
       ## builtin/hash-object.c ##
     -@@ builtin/hash-object.c: static void hash_fd(int fd, const char *type, const char *path, unsigned flags,
     - 	maybe_flush_or_die(stdout, "hash to stdout");
     - }
     - 
     -+#ifdef __MVS__
     -+#  if (__CHARSET_LIB == 1)
     -+#  include <stdio.h>
     -+#  include <stdlib.h>
     -+
     -+   int setbinaryfd(int fd)
     -+   {
     -+     attrib_t attr;
     -+     int rc;
     -+
     -+     memset(&attr, 0, sizeof(attr));
     -+     attr.att_filetagchg = 1;
     -+     attr.att_filetag.ft_ccsid = FT_BINARY;
     -+     attr.att_filetag.ft_txtflag = 0;
     -+
     -+     rc = __fchattr(fd, &attr, sizeof(attr));
     -+     return rc;
     -+   }
     -+#  endif
     -+#endif
     -+
     -+
     - static void hash_object(const char *path, const char *type, const char *vpath,
     - 			unsigned flags, int literally)
     +@@ builtin/hash-object.c: static void hash_object(const char *path, const char *type, const char *vpath,
       {
       	int fd;
       	fd = xopen(path, O_RDONLY);
      +#ifdef __MVS__
     -+#  if (__CHARSET_LIB == 1)
     -+  if (setbinaryfd(fd))
     ++	/*
     ++	 * Since the data being read is in binary format,
     ++	 * we need to disable autoconversion for z/OS
     ++	 */
     ++	if (__setfdbinary(fd))
      +		die_errno("Cannot set to binary '%s'", path);
     -+#  endif
      +#endif
       	hash_fd(fd, type, vpath, flags, literally);
       }
     @@ combine-diff.c: static void show_patch_diff(struct combine_diff_path *elem, int
       			ssize_t done;
       			int is_file, i;
       
     -+#ifdef __MVS__
     -+      __disableautocvt(fd);
     -+#endif
     ++		#ifdef __MVS__
     ++			/*
     ++			 * Disable implicit autconversion on z/os,
     ++			 * rely on conversion from iconv
     ++			 */
     ++			__disableautocvt(fd);
     ++		#endif
      +
       			elem->mode = canon_mode(st.st_mode);
       			/* if symlinks don't work, assume symlink if all parents
       			 * are symlinks
      
     - ## config.c ##
     -@@ config.c: static int git_default_core_config(const char *var, const char *value,
     - 		return 0;
     - 	}
     - 
     -+	#ifdef __MVS__
     -+	if (!strcmp(var, "core.ignorefiletags")) {
     -+		ignore_file_tags = git_config_bool(var, value);
     -+		return 0;
     -+	}
     -+	#endif
     -+
     - 	if (!strcmp(var, "core.safecrlf")) {
     - 		int eol_rndtrp_die;
     - 		if (value && !strcasecmp(value, "warn")) {
     -
     - ## 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
     -
     - ## convert.c ##
     -@@ convert.c: static int check_roundtrip(const char *enc_name)
     - static const char *default_encoding = "UTF-8";
     - 
     - static int encode_to_git(const char *path, const char *src, size_t src_len,
     --			 struct strbuf *buf, const char *enc, int conv_flags)
     -+			 struct strbuf *buf, const char *enc, enum convert_crlf_action attr_action, int conv_flags)
     - {
     - 	char *dst;
     - 	size_t dst_len;
     - 	int die_on_error = conv_flags & CONV_WRITE_OBJECT;
     - 
     -+  if (attr_action == CRLF_BINARY) {
     -+    return 0;
     -+  }
     - 	/*
     - 	 * No encoding is specified or there is nothing to encode.
     - 	 * Tell the caller that the content was not modified.
     -@@ convert.c: static int encode_to_git(const char *path, const char *src, size_t src_len,
     - 		return 0;
     - 
     - 	trace_encoding("source", path, enc, src, src_len);
     -+#ifdef __MVS__
     -+  // Don't convert ISO8859-1 on z/OS
     -+  if (strcasecmp("ISO8859-1", enc) == 0)
     -+    return 0;
     -+#endif
     - 	dst = reencode_string_len(src, src_len, default_encoding, enc,
     - 				  &dst_len);
     - 	if (!dst) {
     -@@ convert.c: static int encode_to_git(const char *path, const char *src, size_t src_len,
     - }
     - 
     - static int encode_to_worktree(const char *path, const char *src, size_t src_len,
     --			      struct strbuf *buf, const char *enc)
     -+			      struct strbuf *buf, enum convert_crlf_action attr_action, const char *enc)
     - {
     - 	char *dst;
     - 	size_t dst_len;
     - 
     -+  if (attr_action == CRLF_BINARY) {
     -+    return 0;
     -+  }
     - 	/*
     - 	 * No encoding is specified or there is nothing to encode.
     - 	 * Tell the caller that the content was not modified.
     -@@ convert.c: static int git_path_check_ident(struct attr_check_item *check)
     - 
     - static struct attr_check *check;
     - 
     -+static const char* get_platform() {
     -+	struct utsname uname_info;
     -+
     -+	if (uname(&uname_info))
     -+		die(_("uname() failed with error '%s' (%d)\n"),
     -+			    strerror(errno),
     -+			    errno);
     -+
     -+  if (!strcmp(uname_info.sysname, "OS/390"))
     -+    return "zos";
     -+  return uname_info.sysname;
     -+}
     -+
     -+
     - void convert_attrs(struct index_state *istate,
     - 		   struct conv_attrs *ca, const char *path)
     - {
     - 	struct attr_check_item *ccheck = NULL;
     -+  struct strbuf platform_working_tree_encoding = STRBUF_INIT;
     -+
     -+	strbuf_addf(&platform_working_tree_encoding, "%s-working-tree-encoding", get_platform());
     -+
     - 
     - 	if (!check) {
     - 		check = attr_check_initl("crlf", "ident", "filter",
     --					 "eol", "text", "working-tree-encoding",
     -+					 "eol", "text", "working-tree-encoding", platform_working_tree_encoding.buf,
     - 					 NULL);
     - 		user_convert_tail = &user_convert;
     - 		git_config(read_convert_config, NULL);
     - 	}
     -+	strbuf_release(&platform_working_tree_encoding);
     - 
     - 	git_check_attr(istate, path, check);
     - 	ccheck = check->items;
     -@@ convert.c: void convert_attrs(struct index_state *istate,
     - 			ca->crlf_action = CRLF_TEXT_CRLF;
     - 	}
     - 	ca->working_tree_encoding = git_path_check_encoding(ccheck + 5);
     -+  if (git_path_check_encoding(ccheck + 6))
     -+    ca->working_tree_encoding = git_path_check_encoding(ccheck + 6);
     - 
     - 	/* Save attr and make a decision for action */
     - 	ca->attr_action = ca->crlf_action;
     -@@ convert.c: int convert_to_git(struct index_state *istate,
     - 		len = dst->len;
     - 	}
     - 
     --	ret |= encode_to_git(path, src, len, dst, ca.working_tree_encoding, conv_flags);
     -+	ret |= encode_to_git(path, src, len, dst, ca.working_tree_encoding, ca.attr_action, conv_flags);
     - 	if (ret && dst) {
     - 		src = dst->buf;
     - 		len = dst->len;
     -@@ convert.c: void convert_to_git_filter_fd(struct index_state *istate,
     - 	if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN, NULL, NULL))
     - 		die(_("%s: clean filter '%s' failed"), path, ca.drv->name);
     - 
     --	encode_to_git(path, dst->buf, dst->len, dst, ca.working_tree_encoding, conv_flags);
     -+	encode_to_git(path, dst->buf, dst->len, dst, ca.working_tree_encoding, ca.attr_action, conv_flags);
     - 	crlf_to_git(istate, path, dst->buf, dst->len, dst, ca.crlf_action, conv_flags);
     - 	ident_to_git(dst->buf, dst->len, dst, ca.ident);
     - }
     -@@ convert.c: static int convert_to_working_tree_ca_internal(const struct conv_attrs *ca,
     - 		}
     - 	}
     - 
     --	ret |= encode_to_worktree(path, src, len, dst, ca->working_tree_encoding);
     -+	ret |= encode_to_worktree(path, src, len, dst, ca->attr_action, ca->working_tree_encoding);
     - 	if (ret) {
     - 		src = dst->buf;
     - 		len = dst->len;
     -
       ## copy.c ##
      @@ copy.c: int copy_fd(int ifd, int ofd)
       		if (write_in_full(ofd, buffer, len) < 0)
       			return COPY_WRITE_ERROR;
       	}
     -+#ifdef __MVS__
     -+  __copyfdccsid(ifd, ofd);
     -+#endif
     ++	#ifdef __MVS__
     ++		/*
     ++		 * This is to ensure that file tags are copied
     ++		 * from source to destination
     ++		 */
     ++		__copyfdccsid(ifd, ofd);
     ++	#endif
       	return 0;
       }
       
      
     - ## diff.c ##
     -@@ diff.c: int diff_populate_filespec(struct repository *r,
     - 	int check_binary = options ? options->check_binary : 0;
     - 	int err = 0;
     - 	int conv_flags = global_conv_flags_eol;
     -+#ifdef __MVS__
     -+	int autocvtToASCII;
     -+#endif
     - 	/*
     - 	 * demote FAIL to WARN to allow inspecting the situation
     - 	 * instead of refusing.
     -@@ diff.c: int diff_populate_filespec(struct repository *r,
     - 			s->is_binary = 1;
     - 			return 0;
     - 		}
     -+#ifdef __MVS__
     -+    validate_codeset(r->index, s->path, &autocvtToASCII);
     -+#endif
     - 		fd = open(s->path, O_RDONLY);
     - 		if (fd < 0)
     - 			goto err_empty;
     -+
     -+#ifdef __MVS__
     -+    if (!autocvtToASCII)
     -+      __disableautocvt(fd);
     -+#endif
     - 		s->data = xmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);
     - 		close(fd);
     - 		s->should_munmap = 1;
     -
     - ## entry.c ##
     -@@ entry.c: int fstat_checkout_output(int fd, const struct checkout *state, struct stat *st)
     - 	return 0;
     + ## fetch-negotiator.h ##
     +@@ fetch-negotiator.h: struct repository;
     +  * Then, when "have" lines are required, call next(). Call ack() to report what
     +  * the server tells us.
     +  *
     +- * Once negotiation is done, call release(). The negotiator then cannot be used
     ++ * Once negotiation is done, call release_negotiator(). The negotiator then cannot be used
     +  * (unless reinitialized with fetch_negotiator_init()).
     +  */
     + struct fetch_negotiator {
     +@@ fetch-negotiator.h: struct fetch_negotiator {
     + 	 */
     + 	int (*ack)(struct fetch_negotiator *, struct commit *);
     + 
     +-	void (*release)(struct fetch_negotiator *);
     ++	void (*release_negotiator)(struct fetch_negotiator *);
     + 
     + 	/* internal use */
     + 	void *data;
     +
     + ## fetch-pack.c ##
     +@@ fetch-pack.c: static struct ref *do_fetch_pack(struct fetch_pack_args *args,
     + 
     +  all_done:
     + 	if (negotiator)
     +-		negotiator->release(negotiator);
     ++		negotiator->release_negotiator(negotiator);
     + 	return ref;
       }
       
     -+#ifdef __MVS__
     -+void tag_file_as_working_tree_encoding(struct index_state *istate, char* path, int fd) {
     -+	struct conv_attrs ca;
     -+	convert_attrs(istate, &ca, path);
     -+  if (ca.attr_action != CRLF_BINARY) {
     -+    if (ca.working_tree_encoding)
     -+      __chgfdcodeset(fd, ca.working_tree_encoding);
     -+    else
     -+      __setfdtext(fd);
     -+  }
     -+  else {
     -+    __setfdbinary(fd);
     -+  }
     -+
     -+  __disableautocvt(fd);
     -+}
     -+#endif
     -+
     - static int streaming_write_entry(const struct cache_entry *ce, char *path,
     - 				 struct stream_filter *filter,
     - 				 const struct checkout *state, int to_tempfile,
     -@@ entry.c: static int streaming_write_entry(const struct cache_entry *ce, char *path,
     - 	if (fd < 0)
     - 		return -1;
     +@@ fetch-pack.c: static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
     + 		die("fsck failed");
       
     -+#ifdef __MVS__
     -+  tag_file_as_working_tree_encoding(state->istate, path, fd);
     -+#endif
     -+
     - 	result |= stream_blob_to_fd(fd, &ce->oid, filter, 1);
     - 	*fstat_done = fstat_checkout_output(fd, state, statbuf);
     - 	result |= close(fd);
     -@@ entry.c: static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca
     - 			return error_errno("unable to create file %s", path);
     - 		}
     + 	if (negotiator)
     +-		negotiator->release(negotiator);
     ++		negotiator->release_negotiator(negotiator);
       
     -+#ifdef __MVS__
     -+    tag_file_as_working_tree_encoding(state->istate, path, fd);
     -+#endif
     -+
     - 		wrote = write_in_full(fd, new_blob, size);
     - 		if (!to_tempfile)
     - 			fstat_done = fstat_checkout_output(fd, state, &st);
     -
     - ## environment.c ##
     -@@ environment.c: const char *git_hooks_path;
     - int zlib_compression_level = Z_BEST_SPEED;
     - int pack_compression_level = Z_DEFAULT_COMPRESSION;
     - int fsync_object_files = -1;
     -+#ifdef __MVS__
     -+int ignore_file_tags = 0;
     -+#endif
     - int use_fsync = -1;
     - enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
     - enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
     + 	oidset_clear(&common);
     + 	return ref;
      
       ## git-compat-util.h ##
      @@ git-compat-util.h: struct strbuf;
     @@ git-compat-util.h: struct strbuf;
       #include <fcntl.h>
       #include <stddef.h>
      +#ifdef __MVS__
     -+#define release stdlib_release
     -+#define fetch stdlib_fetch
     ++	#define release stdlib_release
     ++	#define fetch stdlib_fetch
      +#endif
       #include <stdlib.h>
      +#ifdef __MVS__
     -+#undef fetch
     -+#undef release
     ++	#undef fetch
     ++	#undef release
      +#endif
       #include <stdarg.h>
       #include <string.h>
     @@ negotiator/noop.c: static int ack(struct fetch_negotiator *n UNUSED, struct comm
       }
       
      -static void release(struct fetch_negotiator *n UNUSED)
     -+static void release_negotiator(struct fetch_negotiator *n UNUSED)
     ++static void release_negotiator (struct fetch_negotiator *n UNUSED)
       {
       	/* nothing to release */
       }
     @@ negotiator/skipping.c: void skipping_negotiator_init(struct fetch_negotiator *ne
       	data->rev_list.compare = compare;
       
      
     - ## object-file.c ##
     -@@
     - #include "setup.h"
     - #include "submodule.h"
     - #include "fsck.h"
     --
     -+#ifdef __MVS__
     -+#include <_Ccsid.h>
     -+#endif
     - /* The maximum size for an object header. */
     - #define MAX_HEADER_LEN 32
     - 
     -@@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
     - 	return ret;
     - }
     - 
     -+#ifdef __MVS__
     -+void validate_codeset(struct index_state *istate, const char *path, int* autoconvertToASCII) {
     -+       struct conv_attrs ca;
     -+  struct stat st;
     -+  unsigned short attr_ccsid;
     -+  unsigned short file_ccsid;
     -+
     -+  if (ignore_file_tags)
     -+   return;
     -+
     -+  *autoconvertToASCII = 0;
     -+       convert_attrs(istate, &ca, path);
     -+  if (ca.attr_action == CRLF_BINARY) {
     -+    attr_ccsid = FT_BINARY;
     -+  }
     -+  else if (ca.working_tree_encoding) {
     -+    attr_ccsid = __toCcsid(ca.working_tree_encoding);
     -+  }
     -+  else
     -+    attr_ccsid = 819;
     -+
     -+  if (stat(path, &st) < 0)
     -+    return;
     -+
     -+  file_ccsid = st.st_tag.ft_ccsid;
     -+
     -+  if (file_ccsid == FT_UNTAGGED) {
     -+    die("File %s is untagged, set the correct file tag (using the chtag command).", path);
     -+  }
     -+
     -+  if (attr_ccsid != file_ccsid) {
     -+    if (file_ccsid == 1047 && attr_ccsid == 819) {
     -+      *autoconvertToASCII = 1;
     -+      return;
     -+    }
     -+    // Allow tag mixing of 819 and 1208
     -+    if ((file_ccsid == 819 || file_ccsid == 1208) && (attr_ccsid == 1208 || attr_ccsid == 819)) {
     -+      return;
     -+    }
     -+    // Don't check for binary files, just add them
     -+    if (attr_ccsid == FT_BINARY)
     -+      return;
     -+
     -+    char attr_csname[_XOPEN_PATH_MAX] = {0};
     -+    char file_csname[_XOPEN_PATH_MAX] = {0};
     -+    if (attr_ccsid != FT_BINARY) {
     -+      __toCSName(attr_ccsid, attr_csname);
     -+    } else {
     -+      snprintf(attr_csname, _XOPEN_PATH_MAX, "%s", "binary");
     -+    }
     -+    if (file_ccsid != FT_BINARY) {
     -+      __toCSName(file_ccsid, file_csname);
     -+    } else {
     -+      snprintf(file_csname, _XOPEN_PATH_MAX, "%s", "binary");
     -+    }
     -+    die("%s added file: file tag (%s) does not match working-tree-encoding (%s)", path, file_csname, attr_csname);
     -+  }
     -+}
     -+#endif
     -+
     -+
     -+
     - int index_path(struct index_state *istate, struct object_id *oid,
     - 	       const char *path, struct stat *st, unsigned flags)
     - {
     -@@ object-file.c: int index_path(struct index_state *istate, struct object_id *oid,
     - 	struct strbuf sb = STRBUF_INIT;
     - 	int rc = 0;
     - 
     -+#ifdef __MVS__
     -+	struct conv_attrs ca;
     -+	int autocvtToASCII;
     -+#endif
     -+
     - 	switch (st->st_mode & S_IFMT) {
     - 	case S_IFREG:
     -+#ifdef __MVS__
     -+    validate_codeset(istate, path, &autocvtToASCII);
     -+#endif
     - 		fd = open(path, O_RDONLY);
     - 		if (fd < 0)
     - 			return error_errno("open(\"%s\")", path);
     -+
     -+#ifdef __MVS__
     -+   if (!autocvtToASCII)
     -+     __disableautocvt(fd);
     -+#endif
     -+
     - 		if (index_fd(istate, oid, fd, st, OBJ_BLOB, path, flags) < 0)
     - 			return error(_("%s: failed to insert into database"),
     - 				     path);
     -
       ## read-cache.c ##
      @@ read-cache.c: static int ce_compare_data(struct index_state *istate,
       	int fd = git_open_cloexec(ce->name, O_RDONLY);
       
       	if (fd >= 0) {
     -+#ifdef __MVS__
     -+    __disableautocvt(fd);
     -+#endif
     ++	#ifdef __MVS__
     ++		/*
     ++		 * Since the data is in binary format,
     ++		 * we need to set the z/OS file tag to
     ++		 * binary to disable autoconversion
     ++		 */
     ++		__disableautocvt(fd);
     ++	#endif
       		struct object_id oid;
       		if (!index_fd(istate, &oid, fd, st, OBJ_BLOB, ce->name, 0))
       			match = !oideq(&oid, &ce->oid);
     -
     - ## utf8.c ##
     -@@ utf8.c: char *reencode_string_len(const char *in, size_t insz,
     - #endif
     - 	}
     - 
     -+#ifdef __MVS__
     -+  //HACK: For backwards compat, ISO8859-1 really means utf-8 in the z/OS world
     -+  if (strcasecmp("ISO8859-1", in_encoding) == 0) {
     -+    in_encoding = "UTF-8";
     -+    out_encoding = "UTF-8";
     -+  }
     -+  if (strcasecmp("ISO8859-1", out_encoding) == 0) {
     -+    in_encoding = "UTF-8";
     -+    out_encoding = "UTF-8";
     -+  }
     -+#endif
     - 	conv = iconv_open(out_encoding, in_encoding);
     - 	if (conv == (iconv_t) -1) {
     - 		in_encoding = fallback_encoding(in_encoding);
  3:  e31be0d764f <  -:  ----------- spaces and errors fix Handled git pipeline errors
  4:  7bace397b4a <  -:  ----------- fixes for build errors Handled git pipeline errorse
  5:  3b6d1f80668 <  -:  ----------- fixes for build errors
  6:  3c9b02e18d2 <  -:  ----------- spaces and errors fix Handled git pipeline errors
  7:  8165196f869 <  -:  ----------- spaces and errors fix Handled git pipeline errors
  8:  9fb74d92e3f <  -:  ----------- platform_name fix Handled git pipeline errors
  9:  8fa15ac45f7 <  -:  ----------- strncpy fix Handled git pipeline errors
 10:  63479fe3696 <  -:  ----------- strncpy fix Handled git pipeline errors
 11:  25271363e57 <  -:  ----------- strncpy fix Handled git pipeline errors
 12:  06658ebad10 <  -:  ----------- Handled git pipeline errors - Memory leak
 13:  804624950ae <  -:  ----------- Handled git pipeline errors - z/OS enable


 builtin/archive.c     | 9 +++++++++
 builtin/hash-object.c | 8 ++++++++
 combine-diff.c        | 8 ++++++++
 copy.c                | 7 +++++++
 fetch-negotiator.h    | 4 ++--
 fetch-pack.c          | 4 ++--
 git-compat-util.h     | 8 ++++++++
 negotiator/default.c  | 4 ++--
 negotiator/noop.c     | 4 ++--
 negotiator/skipping.c | 4 ++--
 read-cache.c          | 8 ++++++++
 11 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/builtin/archive.c b/builtin/archive.c
index 90761fdfee0..3b1b258e383 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -14,6 +14,15 @@
 static void create_output_file(const char *output_file)
 {
 	int output_fd = xopen(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
+#ifdef __MVS__
+	/*
+	 * Since the data is in binary format,
+	 * we need to set the z/OS file tag
+	 * to binary to disable autoconversion
+	 */
+	if (__setfdbinary(output_fd))
+		die_errno(_("could not tag archive file '%s'"), output_file);
+#endif
 	if (output_fd != 1) {
 		if (dup2(output_fd, 1) < 0)
 			die_errno(_("could not redirect output"));
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 5ffec99dcea..f43450db02d 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -62,6 +62,14 @@ static void hash_object(const char *path, const char *type, const char *vpath,
 {
 	int fd;
 	fd = xopen(path, O_RDONLY);
+#ifdef __MVS__
+	/*
+	 * Since the data being read is in binary format,
+	 * we need to disable autoconversion for z/OS
+	 */
+	if (__setfdbinary(fd))
+		die_errno("Cannot set to binary '%s'", path);
+#endif
 	hash_fd(fd, type, vpath, flags, literally);
 }
 
diff --git a/combine-diff.c b/combine-diff.c
index f90f4424829..3230b660371 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1082,6 +1082,14 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			ssize_t done;
 			int is_file, i;
 
+		#ifdef __MVS__
+			/*
+			 * Disable implicit autconversion on z/os,
+			 * rely on conversion from iconv
+			 */
+			__disableautocvt(fd);
+		#endif
+
 			elem->mode = canon_mode(st.st_mode);
 			/* if symlinks don't work, assume symlink if all parents
 			 * are symlinks
diff --git a/copy.c b/copy.c
index 23d84c6c1db..f5b9828b1c9 100644
--- a/copy.c
+++ b/copy.c
@@ -14,6 +14,13 @@ int copy_fd(int ifd, int ofd)
 		if (write_in_full(ofd, buffer, len) < 0)
 			return COPY_WRITE_ERROR;
 	}
+	#ifdef __MVS__
+		/*
+		 * This is to ensure that file tags are copied
+		 * from source to destination
+		 */
+		__copyfdccsid(ifd, ofd);
+	#endif
 	return 0;
 }
 
diff --git a/fetch-negotiator.h b/fetch-negotiator.h
index e348905a1f0..71d44102fdc 100644
--- a/fetch-negotiator.h
+++ b/fetch-negotiator.h
@@ -14,7 +14,7 @@ struct repository;
  * Then, when "have" lines are required, call next(). Call ack() to report what
  * the server tells us.
  *
- * Once negotiation is done, call release(). The negotiator then cannot be used
+ * Once negotiation is done, call release_negotiator(). The negotiator then cannot be used
  * (unless reinitialized with fetch_negotiator_init()).
  */
 struct fetch_negotiator {
@@ -47,7 +47,7 @@ struct fetch_negotiator {
 	 */
 	int (*ack)(struct fetch_negotiator *, struct commit *);
 
-	void (*release)(struct fetch_negotiator *);
+	void (*release_negotiator)(struct fetch_negotiator *);
 
 	/* internal use */
 	void *data;
diff --git a/fetch-pack.c b/fetch-pack.c
index 26999e3b659..c1f2e714f8e 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1232,7 +1232,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 
  all_done:
 	if (negotiator)
-		negotiator->release(negotiator);
+		negotiator->release_negotiator(negotiator);
 	return ref;
 }
 
@@ -1853,7 +1853,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 		die("fsck failed");
 
 	if (negotiator)
-		negotiator->release(negotiator);
+		negotiator->release_negotiator(negotiator);
 
 	oidset_clear(&common);
 	return ref;
diff --git a/git-compat-util.h b/git-compat-util.h
index 3e7a59b5ff1..be4516fa64e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -223,7 +223,15 @@ struct strbuf;
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <stddef.h>
+#ifdef __MVS__
+	#define release stdlib_release
+	#define fetch stdlib_fetch
+#endif
 #include <stdlib.h>
+#ifdef __MVS__
+	#undef fetch
+	#undef release
+#endif
 #include <stdarg.h>
 #include <string.h>
 #ifdef HAVE_STRINGS_H
diff --git a/negotiator/default.c b/negotiator/default.c
index 9a5b6963272..b1f9f153372 100644
--- a/negotiator/default.c
+++ b/negotiator/default.c
@@ -174,7 +174,7 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
 	return known_to_be_common;
 }
 
-static void release(struct fetch_negotiator *n)
+static void release_negotiator(struct fetch_negotiator *n)
 {
 	clear_prio_queue(&((struct negotiation_state *)n->data)->rev_list);
 	FREE_AND_NULL(n->data);
@@ -187,7 +187,7 @@ void default_negotiator_init(struct fetch_negotiator *negotiator)
 	negotiator->add_tip = add_tip;
 	negotiator->next = next;
 	negotiator->ack = ack;
-	negotiator->release = release;
+	negotiator->release_negotiator = release_negotiator;
 	negotiator->data = CALLOC_ARRAY(ns, 1);
 	ns->rev_list.compare = compare_commits_by_commit_date;
 
diff --git a/negotiator/noop.c b/negotiator/noop.c
index de39028ab7f..b2d555e0fec 100644
--- a/negotiator/noop.c
+++ b/negotiator/noop.c
@@ -30,7 +30,7 @@ static int ack(struct fetch_negotiator *n UNUSED, struct commit *c UNUSED)
 	return 0;
 }
 
-static void release(struct fetch_negotiator *n UNUSED)
+static void release_negotiator (struct fetch_negotiator *n UNUSED)
 {
 	/* nothing to release */
 }
@@ -41,6 +41,6 @@ void noop_negotiator_init(struct fetch_negotiator *negotiator)
 	negotiator->add_tip = add_tip;
 	negotiator->next = next;
 	negotiator->ack = ack;
-	negotiator->release = release;
+	negotiator->release_negotiator = release_negotiator;
 	negotiator->data = NULL;
 }
diff --git a/negotiator/skipping.c b/negotiator/skipping.c
index 5b91520430c..783b3f27e63 100644
--- a/negotiator/skipping.c
+++ b/negotiator/skipping.c
@@ -243,7 +243,7 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
 	return known_to_be_common;
 }
 
-static void release(struct fetch_negotiator *n)
+static void release_negotiator(struct fetch_negotiator *n)
 {
 	clear_prio_queue(&((struct data *)n->data)->rev_list);
 	FREE_AND_NULL(n->data);
@@ -256,7 +256,7 @@ void skipping_negotiator_init(struct fetch_negotiator *negotiator)
 	negotiator->add_tip = add_tip;
 	negotiator->next = next;
 	negotiator->ack = ack;
-	negotiator->release = release;
+	negotiator->release_negotiator = release_negotiator;
 	negotiator->data = CALLOC_ARRAY(data, 1);
 	data->rev_list.compare = compare;
 
diff --git a/read-cache.c b/read-cache.c
index 080bd39713b..b7189c58144 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -205,6 +205,14 @@ static int ce_compare_data(struct index_state *istate,
 	int fd = git_open_cloexec(ce->name, O_RDONLY);
 
 	if (fd >= 0) {
+	#ifdef __MVS__
+		/*
+		 * Since the data is in binary format,
+		 * we need to set the z/OS file tag to
+		 * binary to disable autoconversion
+		 */
+		__disableautocvt(fd);
+	#endif
 		struct object_id oid;
 		if (!index_fd(istate, &oid, fd, st, OBJ_BLOB, ce->name, 0))
 			match = !oideq(&oid, &ce->oid);

base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] This PR enables a successful git build on z/OS.
  2023-12-04 14:19 ` [PATCH v2] This PR enables a successful git build on z/OS Haritha  via GitGitGadget
@ 2023-12-04 21:46   ` Eric Sunshine
  2023-12-05 20:58     ` René Scharfe
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Sunshine @ 2023-12-04 21:46 UTC (permalink / raw)
  To: Haritha via GitGitGadget; +Cc: git, brian m. carlson, Haritha D

On Mon, Dec 4, 2023 at 9:19 AM Haritha via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> Rename functions like "release" and "fetch"
> due to conflict in z/OS standard C libraries.
> Also disables autoconversion facility on z/OS
> and relies on iconv.
> New files created in binary format are also
> tagged as binary.
>
> Signed-off-by: Haritha D <harithamma.d@ibm.com>
> ---
>     Enabling z/OS workflow for git
>
>     z/OS is an IBM mainframe operating system, also known as OS/390. Our
>     team has been actively involved in porting Git to z/OS and we have made
>     significant modifications to facilitate this process. The patch below is
>     the initial configuration for z/OS. I also have few follow up changes
>     and I will send that after these changes are approved. Please let me
>     know if there are any concerns.

It's fairly unlikely that this patch will be accepted as-is. Please
see brian's[1] and Junio's[2] valuable review comments in response to
your v1. They contain important suggestions which will give you a
better chance of landing these changes.

Generally speaking, the patch's commit message lacks sufficient detail
to allow a reviewer (or future reader) to understand why the changes
are being made. Moreover, this single patch seems to be addressing at
least three separate issues, hence should be split into three or more
patches, each standalone and tackling a single issue, and each easily
digested by a reviewer. The commit message of each patch should fully
explain and justify the changes made by the patch, keeping in mind
that most reviewers probably aren't familiar with z/OS, thus will need
extra hand-holding. More below...

[1]: https://lore.kernel.org/git/ZVKrWSv7JguKTSYw@tapette.crustytoothpaste.net/
[2]: https://lore.kernel.org/git/xmqqcywd2m9i.fsf@gitster.g/

> diff --git a/builtin/archive.c b/builtin/archive.c
> @@ -14,6 +14,15 @@
>  static void create_output_file(const char *output_file)
>  {
>         int output_fd = xopen(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
> +#ifdef __MVS__
> +       /*
> +        * Since the data is in binary format,
> +        * we need to set the z/OS file tag
> +        * to binary to disable autoconversion
> +        */
> +       if (__setfdbinary(output_fd))
> +               die_errno(_("could not tag archive file '%s'"), output_file);
> +#endif

As mentioned in an earlier review, the project generally doesn't want
#ifdef's littering the code and prefer that this sort of
platform-specific specialization be wrapped up in its own "compat"
file/function. For instance, perhaps you could create a
platform-specific specialization of xopen() and then `#define xopen`
to reference your specialized version. Your custom xopen() might first
call the xopen() which Git defines and then perform whatever extra
special work is needed for your platform. That way, you would not have
to muck around either in the code which calls xopen() or in the
Git-supplied xopen(). See, for example, how git-compat-util.h
overriedes certain functions, such as stat(), fstat(), etc. using an
#undefine/#define dance.

> diff --git a/combine-diff.c b/combine-diff.c
> @@ -1082,6 +1082,14 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
> +               #ifdef __MVS__
> +                       /*
> +                        * Disable implicit autconversion on z/os,
> +                        * rely on conversion from iconv
> +                        */
> +                       __disableautocvt(fd);
> +               #endif
>                         elem->mode = canon_mode(st.st_mode);

Similar comment. Try to find an abstraction which allows you to
perform this specialization in a way which does not require #ifdef's
within the main source code if possible.

> diff --git a/fetch-negotiator.h b/fetch-negotiator.h
> @@ -47,7 +47,7 @@ struct fetch_negotiator {
> -       void (*release)(struct fetch_negotiator *);
> +       void (*release_negotiator)(struct fetch_negotiator *);> diff --git a/fetch-pack.c b/fetch-pack.c
> @@ -1232,7 +1232,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
>         if (negotiator)
> -               negotiator->release(negotiator);
> +               negotiator->release_negotiator(negotiator);
>         return ref;
>  }
> diff --git a/git-compat-util.h b/git-compat-util.h
> @@ -223,7 +223,15 @@ struct strbuf;
> +#ifdef __MVS__
> +       #define release stdlib_release
> +       #define fetch stdlib_fetch
> +#endif
>  #include <stdlib.h>
> +#ifdef __MVS__
> +       #undef fetch
> +       #undef release
> +#endif

So, the problem is that z/OS is polluting the C namespace or the
preprocessor namespace with names "release" and "fetch"? When we've
run across this problem on other platforms, we modify
git-compat-util.h or some other files in compat/ to suppress the
pollution introduced by the platform headers rather than "fixing" the
Git source code. For instance, if "release" and "fetch" are macros on
z/OS, then you may be able to simply #undef them after pulling in
whichever z/OS header defines them. If they are actual system
functions (rather than macros), you may be able to employ the
#undef/#define dance to rename them to something else, such as
"zos_release" and "zos_fetch" _before_ including the system header
which declares those functions.

> diff --git a/read-cache.c b/read-cache.c
> @@ -205,6 +205,14 @@ static int ce_compare_data(struct index_state *istate,
>         int fd = git_open_cloexec(ce->name, O_RDONLY);
>         if (fd >= 0) {
> +       #ifdef __MVS__
> +               /*
> +                * Since the data is in binary format,
> +                * we need to set the z/OS file tag to
> +                * binary to disable autoconversion
> +                */
> +               __disableautocvt(fd);
> +       #endif

Same comment as above about encapsulating this in a platform-specific
specialization function in compat/ rather than polluting the code with
#ifdef.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] This PR enables a successful git build on z/OS.
  2023-12-04 21:46   ` Eric Sunshine
@ 2023-12-05 20:58     ` René Scharfe
  0 siblings, 0 replies; 21+ messages in thread
From: René Scharfe @ 2023-12-05 20:58 UTC (permalink / raw)
  To: Eric Sunshine, Haritha via GitGitGadget; +Cc: git, brian m. carlson, Haritha D

Am 04.12.23 um 22:46 schrieb Eric Sunshine:
> On Mon, Dec 4, 2023 at 9:19 AM Haritha via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> diff --git a/fetch-negotiator.h b/fetch-negotiator.h
>> @@ -47,7 +47,7 @@ struct fetch_negotiator {
>> -       void (*release)(struct fetch_negotiator *);
>> +       void (*release_negotiator)(struct fetch_negotiator *);> diff --git a/fetch-pack.c b/fetch-pack.c
>> @@ -1232,7 +1232,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
>>         if (negotiator)
>> -               negotiator->release(negotiator);
>> +               negotiator->release_negotiator(negotiator);
>>         return ref;
>>  }
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> @@ -223,7 +223,15 @@ struct strbuf;
>> +#ifdef __MVS__
>> +       #define release stdlib_release
>> +       #define fetch stdlib_fetch
>> +#endif
>>  #include <stdlib.h>
>> +#ifdef __MVS__
>> +       #undef fetch
>> +       #undef release
>> +#endif
>
> So, the problem is that z/OS is polluting the C namespace or the
> preprocessor namespace with names "release" and "fetch"? When we've
> run across this problem on other platforms, we modify
> git-compat-util.h or some other files in compat/ to suppress the
> pollution introduced by the platform headers rather than "fixing" the
> Git source code. For instance, if "release" and "fetch" are macros on
> z/OS, then you may be able to simply #undef them after pulling in
> whichever z/OS header defines them. If they are actual system
> functions (rather than macros), you may be able to employ the
> #undef/#define dance to rename them to something else, such as
> "zos_release" and "zos_fetch" _before_ including the system header
> which declares those functions.

I assume that [1] and [2] link to the documentation of these functions.
Both pages include the following paragraph:

   "To avoid infringing on the user's name space, this nonstandard
    function has two names. One name is prefixed with two underscore
    characters, and one name is not. The name without the prefix
    underscore characters is exposed only when using the runtime library
    extensions."

[3] defines "runtime library extensions" and mentions the macro __EXT
and LANGLVL(EXTENDED).  Do you need those extensions?  If you don't then
perhaps turning them off avoids the name collisions without needing to
change the code?

René


[1] https://www.ibm.com/docs/en/zos/3.1.0?topic=functions-fetch-get-load-module
[2] https://www.ibm.com/docs/en/zos/3.1.0?topic=functions-release-delete-load-module
[3] https://www.ibm.com/docs/en/zos/3.1.0?topic=reference-zos-cc-compiler-feature#compiler_feature__ext_lib_func


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-12-05 20:58 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-13 10:24 [PATCH 00/13] Enabling z/OS workflow for git Haritha  via GitGitGadget
2023-11-13 10:24 ` [PATCH 01/13] " Haritha D via GitGitGadget
2023-11-13 10:24 ` [PATCH 02/13] Enable builds for z/OS Haritha D via GitGitGadget
2023-11-13 23:03   ` brian m. carlson
2023-11-14  1:47     ` Junio C Hamano
2023-11-14  0:48   ` Junio C Hamano
2023-11-13 10:24 ` [PATCH 03/13] spaces and errors fix Handled git pipeline errors Haritha D via GitGitGadget
2023-11-14  0:38   ` Junio C Hamano
2023-11-13 10:24 ` [PATCH 04/13] fixes for build errors Handled git pipeline errorse Haritha D via GitGitGadget
2023-11-13 10:24 ` [PATCH 05/13] fixes for build errors Haritha D via GitGitGadget
2023-11-13 10:24 ` [PATCH 06/13] spaces and errors fix Handled git pipeline errors Haritha D via GitGitGadget
2023-11-13 10:24 ` [PATCH 07/13] " Haritha D via GitGitGadget
2023-11-13 10:24 ` [PATCH 08/13] platform_name " Haritha D via GitGitGadget
2023-11-13 10:24 ` [PATCH 09/13] strncpy " Haritha D via GitGitGadget
2023-11-13 10:24 ` [PATCH 10/13] " Haritha D via GitGitGadget
2023-11-13 10:24 ` [PATCH 11/13] " Haritha D via GitGitGadget
2023-11-13 10:24 ` [PATCH 12/13] Handled git pipeline errors - Memory leak Haritha D via GitGitGadget
2023-11-13 10:24 ` [PATCH 13/13] Handled git pipeline errors - z/OS enable Haritha D via GitGitGadget
2023-12-04 14:19 ` [PATCH v2] This PR enables a successful git build on z/OS Haritha  via GitGitGadget
2023-12-04 21:46   ` Eric Sunshine
2023-12-05 20:58     ` René Scharfe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).