Enabling z/OS workflow for git
cover 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.
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
Signature · 1 line
gitgitgadget
PATCH 1/13 Enabling z/OS workflow for git
From: Haritha D <[email protected]>
Our team ported Git to z/OS w/ significant modifcation. This patch is Initial Config
--- config.mak.uname | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
config.mak.uname +20 -0
@@ -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")
Signature · 1 line
gitgitgadget
PATCH 2/13 Enable builds for z/OS.
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
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.
--- 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(-)
Makefile +16 -5
@@ -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' \ [email protected] >$@+ 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 '}' \
builtin.h +3 -0
@@ -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
builtin/archive.c +6 -0
@@ -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" ));
builtin/hash-object.c +28 -0
@@ -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); }
combine-diff.c +4 -0
@@ -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
config.c +7 -0
@@ -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" )) {
configure.ac +3 -0
@@ -463,6 +463,9 @@ else CC_LD_DYNPATH=-Wl,+b, else CC_LD_DYNPATH= + if test "$(uname -s)" = "OS/390"; then + CC_LD_DYNPATH=-L + fi AC_MSG_WARN([linker does not support runtime path to dynamic libraries]) fi fi
convert.c +38 -6
112 lines not on this page · raw message
copy.c +3 -0
@@ -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.c +11 -0
@@ -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 ;
entry.c +26 -0
@@ -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 *careturn 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);
environment.c +3 -0
@@ -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 ;
git-compat-util.h +8 -0
@@ -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
negotiator/default.c +2 -2
@@ -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;
negotiator/noop.c +2 -2
@@ -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 ; }
negotiator/skipping.c +2 -2
@@ -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;
object-file.c +79 -1
103 lines not on this page · raw message
read-cache.c +3 -0
@@ -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);
utf8.c +11 -0
@@ -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);
Signature · 1 line
gitgitgadget
Re: [PATCH 02/13] Enable builds for z/OS.
Quoted 6 lines · Haritha D via GitGitGadget
From: Haritha D <[email protected]>
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.
Quoted 4 lines
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.
Quoted 42 lines
Also adds support for scripts that are not in standard locations so that
/bin/envbash can be specified.Signed-off-by Harithamma D <[email protected]>--- 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(-)Makefile +2
-0@@ -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. #
Makefile include shared.mak · line 220
Quoted 4 lines
Makefile +1
-0@@ -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.
Quoted 5 lines
builtin/archive.c +0
-0
builtin/archive.c line 22
Quoted 9 lines
builtin/archive.c +6
-0@@ -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.
Quoted 8 lines
builtin/archive.c +0
-0if (output_fd !=1 ) {if (dup2 (output_fd,1 ) <0 )die_errno (_ ("could not redirect output" ));builtin/hash-object.c +0
-0
builtin/hash-object.c static void hash_fd(int fd, const char *type, const char *path, unsigned flags, · line 63
Quoted 7 lines
builtin/hash-object.c +4
-0@@ -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.
Quoted 5 lines
builtin/hash-object.c +5
-0+int setbinaryfd (int fd) + { +attrib_t attr; +int rc; +
builtin/hash-object.c static void hash_fd(int fd, const char *type, const char *path, unsigned flags, · line 79
Quoted 10 lines
builtin/hash-object.c +10
-0+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.
combine-diff.c static void show_patch_diff(struct combine_diff_path *elem, int num_parent, · line 1087
Quoted 11 lines
combine-diff.c +3
-0@@ -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.
Quoted 5 lines
config.c +0
-0
config.c static int git_default_core_config(const char *var, const char *value, · line 1529
Quoted 9 lines
config.c +6
-0@@ -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.
Quoted 8 lines
convert.c +0
-0@@ -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,
convert.c static int check_roundtrip(const char *enc_name) · line 388
Quoted 10 lines
convert.c +4
-1- 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.
convert.c static int encode_to_git(const char *path, const char *src, size_t src_len, · line 413
Quoted 12 lines
convert.c +5
-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
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.
convert.c static int git_path_check_ident(struct attr_check_item *check) · line 1327
Quoted 12 lines
convert.c +12
-0+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.
Signature · 2 lines
brian m. carlson (he/him or they/them) Toronto, Ontario, CA
Re: [PATCH 02/13] Enable builds for z/OS.
Quoted 13 lines · brian m. carlson
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.
Quoted 6 lines
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.
Re: [PATCH 02/13] Enable builds for z/OS.
In reply to Haritha D via GitGitGadgetQuoted 1 line · Haritha D via GitGitGadget
Subject: Re: [PATCH 02/13] Enable builds for z/OS.
Documentation/CodingGuidelines and Documentation/SubmittingPatches would help here, I think.
Quoted 3 lines
# Define
SHELL_PATHto a POSIX shell if your/bin/shis broken. #
+# DefineSHELL_PATH_FOR_SCRIPTSto a POSIX shell if your/bin/shis 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.
Quoted 7 lines
builtin/hash-object.c +0
-0@@ -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" ); }
builtin/hash-object.c static void hash_fd(int fd, const char *type, const char *path, unsigned flags, · line 68
Quoted 10 lines
builtin/hash-object.c +9
-0+#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.
PATCH 3/13 spaces and errors fix Handled git pipeline errors
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This PR has fixes to enable build on z/OS
--- builtin/hash-object.c | 6 +++--- convert.c | 22 +++++++++++++++++----- entry.c | 22 +++++++++++----------- object-file.c | 6 +++--- 4 files changed, 34 insertions(+), 22 deletions(-)
builtin/hash-object.c +3 -3
@@ -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
convert.c +17 -5
@@ -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,
entry.c +11 -11
@@ -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
object-file.c +3 -3
@@ -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 )
Signature · 1 line
gitgitgadget
Re: [PATCH 03/13] spaces and errors fix Handled git pipeline errors
Quoted 1 line · Haritha D via GitGitGadget
Subject: Re: [PATCH 03/13] spaces and errors fix Handled git pipeline errors
-ECANNOTPARSE. Perhaps Documentation/CodingGuidelines and
Documentation/SubmittingPatches may help?
Quoted 3 lines
From: Haritha D <[email protected]>
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.
Quoted 16 lines
builtin/hash-object.c +2
-2@@ -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;
builtin/hash-object.c static void hash_fd(int fd, const char *type, const char *path, unsigned flags, · line 79
Quoted 7 lines
builtin/hash-object.c +1
-1@@ -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?
Quoted 10 lines
convert.c +2
-0@@ -1315,15 +1315,28 @@ static struct attr_check *check;static const char *get_platform () {struct utsname uname_info; +char *result; +if (!uname_info.sysname)
convert.c static struct attr_check *check; · line 1329
Quoted 10 lines
convert.c +10
-0+ { + 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.
PATCH 4/13 fixes for build errors Handled git pipeline errorse
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This PR has fixes to enable build on z/OS
--- convert.c | 4 ++-- fetch-negotiator.h | 2 +- fetch-pack.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
convert.c +2 -2
@@ -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];
fetch-negotiator.h +1 -1
@@ -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;
fetch-pack.c +1 -1
@@ -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; }
Signature · 1 line
gitgitgadget
PATCH 5/13 fixes for build errors
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This patch has fixes for build errors
--- convert.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
convert.c +2 -2
@@ -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 );
Signature · 1 line
gitgitgadget
PATCH 6/13 spaces and errors fix Handled git pipeline errors
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This PR has fixes to enable build on z/OS
--- convert.c | 2 +- fetch-pack.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
convert.c +1 -1
@@ -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 ]) {
fetch-pack.c +1 -1
@@ -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;
Signature · 1 line
gitgitgadget
PATCH 7/13 spaces and errors fix Handled git pipeline errors
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This PR has fixes to enable build on z/OS
--- convert.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
convert.c +11 -7
@@ -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; }
Signature · 1 line
gitgitgadget
PATCH 8/13 platform_name fix Handled git pipeline errors
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This PR has fixes to enable build on z/OS
--- convert.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
convert.c +3 -8
@@ -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" ))
Signature · 1 line
gitgitgadget
PATCH 9/13 strncpy fix Handled git pipeline errors
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This PR has fixes to enable build on z/OS
--- convert.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
convert.c +7 -1
@@ -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
Signature · 1 line
gitgitgadget
PATCH 10/13 strncpy fix Handled git pipeline errors
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This PR has fixes to enable build on z/OS
--- convert.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
convert.c +3 -7
@@ -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; }
Signature · 1 line
gitgitgadget
PATCH 11/13 strncpy fix Handled git pipeline errors
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This PR has fixes to enable build on z/OS
--- convert.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
convert.c +2 -1
@@ -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++; } }
Signature · 1 line
gitgitgadget
PATCH 12/13 Handled git pipeline errors - Memory leak
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This PR has fixes to enable build on z/OS
--- convert.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
convert.c +10 -8
@@ -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",
Signature · 1 line
gitgitgadget
PATCH 13/13 Handled git pipeline errors - z/OS enable
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
This PR has fixes to enable build on z/OS
--- convert.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
convert.c +2 -4
@@ -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",
Signature · 1 line
gitgitgadget
PATCH v2 1/1 This PR enables a successful git build on z/OS.
In reply to Haritha via GitGitGadgetFrom: Haritha D <[email protected]>
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.
---
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 <[email protected]>
## 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 <[email protected]>
-
- ## 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' \
- [email protected] >$@+
- 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 <[email protected]>
## 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 enablebuiltin/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(-)
builtin/archive.c +9 -0
@@ -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" ));
builtin/hash-object.c +8 -0
@@ -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); }
combine-diff.c +8 -0
@@ -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
copy.c +7 -0
@@ -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 ; }
fetch-negotiator.h +2 -2
@@ -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;
fetch-pack.c +2 -2
@@ -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;
git-compat-util.h +8 -0
@@ -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
negotiator/default.c +2 -2
@@ -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;
negotiator/noop.c +2 -2
@@ -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 ; }
negotiator/skipping.c +2 -2
@@ -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;
read-cache.c +8 -0
@@ -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
Signature · 1 line
gitgitgadget
Re: [PATCH v2] This PR enables a successful git build on z/OS.
Quoted 11 lines · Haritha via GitGitGadget
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 <[email protected]>---
Enabling z/OS workflow for git
Quoted 6 lines
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/[email protected]/
[2]: https://lore.kernel.org/git/[email protected]/
Quoted 4 lines
builtin/archive.c +0
-0@@ -14,6 +14,15 @@static void create_output_file (const char *output_file) {
builtin/archive.c line 25
Quoted 10 lines
builtin/archive.c +9
-0int 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.
combine-diff.c static void show_patch_diff(struct combine_diff_path *elem, int num_parent, · line 1089
Quoted 10 lines
combine-diff.c +7
-0@@ -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.
Quoted 12 lines
fetch-negotiator.h +2
-2@@ -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; }
git-compat-util.h struct strbuf; · line 231
Quoted 9 lines
git-compat-util.h +8
-0@@ -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.
read-cache.c static int ce_compare_data(struct index_state *istate, · line 214
Quoted 12 lines
read-cache.c +8
-0@@ -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.
Re: [PATCH v2] This PR enables a successful git build on z/OS.
Am 04.12.23 um 22:46 schrieb Eric Sunshine:
Quoted 28 lines
Quoted 12 lines · Haritha via GitGitGadget
fetch-negotiator.h +2
-2@@ -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; }git-compat-util.h struct strbuf; · line 231
Quoted 9 lines
git-compat-util.h +8
-0@@ -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.hor some other files in compat/ to suppress the
Quoted 8 lines
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