Skip to content

skip tagging the file#176

Open
HarithaIBM wants to merge 9 commits intozopencommunity:mainfrom
HarithaIBM:main
Open

skip tagging the file#176
HarithaIBM wants to merge 9 commits intozopencommunity:mainfrom
HarithaIBM:main

Conversation

@HarithaIBM
Copy link
Copy Markdown
Member

with the target encoding if the conversion fails

@HarithaIBM
Copy link
Copy Markdown
Member Author

In recent build only 92 tests were running
https://cicd.zopen.community/job/Port-Build/9643/consoleFull

Fix is to enable tests - this is WIP , to enable all tests..

@HarithaIBM HarithaIBM marked this pull request as ready for review April 13, 2026 10:51
@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Apr 13, 2026

🤖 Augment PR Summary

Summary: Improves z/OS handling for Git’s working-tree encoding/tagging behavior and expands build/runtime dependencies to support iconv-based conversions.

Changes:

  • Adds libiconv to build and runtime deps and wires --with-libiconv-prefix into configure flags.
  • Introduces/extends z/OS-specific file descriptor handling (binary mode + disabling auto-conversion) across multiple code paths (apply, archive, index-pack, pack-write, run-command pipes, etc.).
  • Enhances encoding conversion logic to better handle EBCDIC newlines and adds best-effort conversion behavior (incl. optional TRANSLIT) with improved error diagnostics.
  • Adds z/OS-specific tagging support for files written to the working tree, including behavior when conversion fails.
  • Adds lockfile tagging support via a new LOCK_TAG_TEXT flag and uses it for config lockfiles.
  • Adds a new z/OS encoding regression test (t0082-zos-encoding.sh) and enables it in Meson test lists.

Technical Notes: Several changes rely on z/OS-specific APIs (__setfdbinary, __disableautocvt, __chgfdccsid) and new config/env controls (core.utf8ccsid, core.iconvtranslit, GIT_UTF8_CCSID).

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 4 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

+ line++;
+ col = 1;
+ } else {
+ col++;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In find_first_non_ascii(), col is incremented before checking whether src[i] is the offending byte, so the reported column will be off-by-one (e.g., a bad first byte reports col 2). This likely makes the new test that expects col 1 fail and also makes diagnostics misleading.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

+ TEST_SHELL_PATH = $(SHELL_PATH)
+ SHELL_PATH_FOR_SCRIPTS = /bin/env bash
+ PYTHON_PATH = python
+ PYTHON_PATH = /home/opnzos/local/pyz/bin/python
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PYTHON_PATH is hard-coded to /home/opnzos/local/pyz/bin/python, which is likely environment-specific and can break builds/tests on other z/OS setups where Python is installed elsewhere.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment thread buildenv
export ZOPEN_INSTALL_OPTS=""
#export ZOPEN_CHECK_OPTS="-i test -j\$ZOPEN_NUM_JOBS"
export ZOPEN_CHECK_OPTS="test -j\$ZOPEN_NUM_JOBS"
export ZOPEN_CHECK_OPTS="-i test -j\$ZOPEN_NUM_JOBS"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZOPEN_CHECK_OPTS uses make -i test ..., which ignores test failures; this can cause CI/package builds to look successful even when the test suite is failing, making regressions easy to miss.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

+ __chgfdccsid(lk->tempfile->fd, 819);
+ struct stat st;
+ if (lk->tempfile && fstat(lk->tempfile->fd, &st) >= 0 && S_ISREG(st.st_mode)) {
+ if (flags & LOCK_TAG_TEXT)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On z/OS the lockfile fd is tagged (text) or set binary, but auto-conversion isn’t disabled here; that’s inconsistent with most other z/OS fd handling in this PR and could allow unintended codepage conversion while writing lockfile contents.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant