Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 33 additions & 25 deletions .github/workflows/zos-py-build.yml

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems like the time budget could be a bit tight here for timing out the workflow (currently, the workflow is set to time out after 20min) - should we consider increasing the timeout in this PR?

Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

permissions:
contents: read
pull-requests: write
checks: write

jobs:
build-python:
Expand Down Expand Up @@ -55,14 +55,26 @@
# Keep in step with zos-build.yml: Open XL C/C++ 2.1 is the newest compiler IBM
# supports on z/OS 2.5, our minimum supported release. See
# native/c/compat/README.md and https://github.com/zowe/zowex/issues/871.
preBuildCmd: "export PATH=/usr/lpp/IBM/cnw/v2r1/openxl/bin:\$PATH"
# /ZOWE/tmp/zowex/python/swig is where Path B (see below) installs a
# precompiled SWIG when the remote system doesn't have one; the Lib/
# check keeps SWIG_LIB unset on Path A so the Makefile's own
# `swig -swiglib` fallback resolves the system SWIG instead.
preBuildCmd: "export PATH=/usr/lpp/IBM/cnw/v2r1/openxl/bin:/ZOWE/tmp/zowex/python/swig:\$PATH\nif [ -d /ZOWE/tmp/zowex/python/swig/Lib ]; then export SWIG_LIB=/ZOWE/tmp/zowex/python/swig/Lib; fi"
EOF

- name: Set up test fixtures
run: |
cat << EOF > native/python/bindings/test/fixtures/env.yml
OWNER: ${{ secrets.SSH_MARIST_ZNP_ID }}
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
DSN_PREFIX: ${{ secrets.SSH_MARIST_ZNP_ID }}.PY#BI
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
USS_BASE_DIR: /ZOWE/tmp/test_uss_functions_for_python_bindings
EOF

- name: Install Dependencies
run: npm ci --workspaces=false

- name: Upload Python Bindings
run: npm run z:upload
- name: Build on z/OS
run: npm run z:rebuild

# SWIG is required to build the bindings from source. It is not always
# available on the remote z/OS system, so check first and branch on it.
Expand All @@ -75,33 +87,29 @@
echo "present=false" >> "$GITHUB_OUTPUT"
fi

# Path A: SWIG present -> build the bindings from source as usual.
- name: Build Python Bindings
if: steps.swig.outputs.present == 'true'
run: npm run z:python:build

# Path B: SWIG absent -> fetch the latest precompiled bundle posted to this
# PR (any author) and apply it so the tests can run without building.
- name: Fetch precompiled Python bindings
# Path B: SWIG absent -> download the precompiled SWIG binary published to
# the zowex releases and install it under python/swig on z/OS, where
# preBuildCmd (above) puts it on PATH for the build below to find.
- name: Fetch and install SWIG from GitHub Releases
if: steps.swig.outputs.present != 'true'
env:
GH_TOKEN: ${{ github.token }}
run: npm run z:python:fetch -- ${{ github.event.pull_request.number }}
run: npm run z:python:swig:install

- name: Apply precompiled Python bindings
if: steps.swig.outputs.present != 'true' && hashFiles('dist/zbind_bin_dist.tar.gz') != ''
run: npm run z:python:apply

- name: Warn if no bindings available
if: steps.swig.outputs.present != 'true' && hashFiles('dist/zbind_bin_dist.tar.gz') == ''
run: echo "::warning::SWIG not found on z/OS and no 'Precompiled Python bindings' comment found on this PR — skipping Python tests."
# Build the bindings from source. On Path A this uses the system SWIG; on
# Path B, the one just installed above.
- name: Build Python Bindings
run: npm run z:python:build

# Run tests when we have bindings: either freshly built, or applied from a
# downloaded precompiled bundle.
- name: Test Python Bindings
if: steps.swig.outputs.present == 'true' || hashFiles('dist/zbind_bin_dist.tar.gz') != ''
run: npm run z:python:test

- name: Python Test Report
uses: dorny/test-reporter@v2

Check failure on line 106 in .github/workflows/zos-py-build.yml

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use full commit SHA hash for this dependency.

See more on https://sonarcloud.io/project/issues?id=zowe_zowe-native-proto&issues=AaBIn8ZRCcDRN4BVDXTS&open=AaBIn8ZRCcDRN4BVDXTS&pullRequest=1121
if: ${{ !cancelled() }}
with:
name: Python binding test results
path: "native/python/bindings/test/pybi_results.xml"
reporter: java-junit

- name: Clean up on z/OS
if: always()
run: npm run z:delete
4 changes: 4 additions & 0 deletions native/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how

## Recent Changes

- `python`: Fixed the Python bindings compiling the shared `native/c` sources with `-fzos-le-char-mode=ascii`, which flipped their execution charset to ASCII and broke every data set and job binding. [#1121](https://github.com/zowe/zowex/pull/1121)
- `python`: `list_data_sets` accepts `show_attributes` so `dsorg`, `volser`, `recfm` and `migrated` can be populated. [#1121](https://github.com/zowe/zowex/pull/1121)
- `python`: Fixed `get_job_status` returning unconverted EBCDIC strings and `create_data_set` raising errors with an empty message. [#1121](https://github.com/zowe/zowex/pull/1121)
- `python`: Fixed `list_jobs_by_owner` failing when no job name prefix is given. [#1121](https://github.com/zowe/zowex/pull/1121)
- `c`: Added JSON-RPC support for listing parmlib data sets. [#1124](https://github.com/zowe/zowex/pull/1124)
- `c`: Added the `zowex ds resolve-alias <dsn>` command which allows the user to resolve data set aliases. [#1108](https://github.com/zowe/zowex/pull/1108)
- `c`: Added `zds_idcams` utility function which allows the caller to execute the z/OS IDCAMS program and retrieve its output. [#1108](https://github.com/zowe/zowex/pull/1108)
Expand Down
2 changes: 1 addition & 1 deletion native/c/zjb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -879,12 +879,12 @@ int zjb_view(ZJB *zjb, const std::string &jobid, ZJob &job)
return RTNCD_SUCCESS;
}

#ifndef SWIG // Convenience overloads are not declared for SWIG; see zjb.hpp
int zjb_list_by_owner(ZJB *zjb, const std::string &owner_name, std::vector<ZJob> &jobs)
{
return zjb_list_by_owner(zjb, owner_name, "", "", jobs);
}

#ifndef SWIG // Exclude owner and prefix implementation for SWIG
int zjb_list_by_owner(ZJB *zjb, const std::string &owner_name, const std::string &prefix_name, std::vector<ZJob> &jobs)
{
return zjb_list_by_owner(zjb, owner_name, prefix_name, "", jobs);
Expand Down
16 changes: 10 additions & 6 deletions native/c/zjb.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ struct ZJobDD
bool is_asa = false;
};

// The convenience overloads are hidden from SWIG: C linkage cannot express an overload set, and
// every symbol crossing the bindings boundary needs it. See the extern "C" block below.
#ifndef SWIG
/**
* @brief Return a list of jobs from an input or default owner
*
Expand All @@ -55,7 +58,6 @@ struct ZJobDD
*/
int zjb_list_by_owner(ZJB *zjb, const std::string &owner_name, std::vector<ZJob> &jobs);

#ifndef SWIG
/**
* @brief Return a list of jobs from an input or default owner
*
Expand All @@ -69,6 +71,13 @@ int zjb_list_by_owner(ZJB *zjb, const std::string &owner_name, std::vector<ZJob>
int zjb_list_by_owner(ZJB *zjb, const std::string &owner_name, const std::string &prefix_name, std::vector<ZJob> &jobs);
#endif

// The bindings compile this header EBCDIC and their SWIG wrappers ASCII. libc++ uses a distinct
// inline namespace per char mode (std::__1 vs std::__1_a), so a mangled name is unresolvable
// across that boundary -- everything the bindings call needs C linkage.
#ifdef SWIG
extern "C"
{
#endif
/**
* @brief Return a list of jobs from an input or default owner
*
Expand All @@ -82,11 +91,6 @@ int zjb_list_by_owner(ZJB *zjb, const std::string &owner_name, const std::string
*/
int zjb_list_by_owner(ZJB *zjb, const std::string &owner_name, const std::string &prefix_name, const std::string &status_name, std::vector<ZJob> &jobs);

// Exclude status implementation for SWIG
#ifdef SWIG
extern "C"
{
#endif
/**
* @brief Return a list of proclib for a job
*
Expand Down
13 changes: 12 additions & 1 deletion native/c/zusf.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,16 @@ struct ZusfListEntry
std::string mtime;
};

// The Python bindings compile this header EBCDIC and their SWIG wrappers ASCII. libc++ uses a
// distinct inline namespace per char mode (std::__1 vs std::__1_a), so a mangled name is
// unresolvable across that boundary -- everything the bindings call needs C linkage.
#ifdef SWIG
extern "C"
{
#endif
int zusf_copy_file_or_dir(ZUSF *zusf, const std::string &source_fs, const std::string &dest_fs, const CopyOptions &options);
int zusf_create_uss_file_or_dir(ZUSF *zusf, const std::string &file, mode_t mode, const CreateOptions &options);
int zusf_move_uss_file_or_dir(ZUSF *zusf, const std::string &source, const std::string &target, bool force = true);
std::string zusf_format_file_entry(ZUSF *zusf, const struct stat &file_stats, const std::string &file_path, const std::string &display_name, ListOptions options, bool use_csv_format, std::vector<ZusfListEntry> *entries = nullptr);
int zusf_list_uss_file_path(ZUSF *zusf, const std::string &file, std::string &response, ListOptions options = ListOptions{}, bool use_csv_format = false, std::vector<ZusfListEntry> *entries = nullptr);
int zusf_read_from_uss_file(ZUSF *zusf, const std::string &file, std::string &response);
int zusf_read_from_uss_file_streamed(ZUSF *zusf, const std::string &file, const std::string &pipe, size_t *content_len);
Expand All @@ -84,12 +90,17 @@ int zusf_chmod_uss_file_or_dir(ZUSF *zusf, const std::string &file, mode_t mode,
int zusf_delete_uss_item(ZUSF *zusf, const std::string &file, bool recursive);
int zusf_chown_uss_file_or_dir(ZUSF *zusf, const std::string &file, const std::string &owner, bool recursive);
int zusf_chtag_uss_file_or_dir(ZUSF *zusf, const std::string &file, const std::string &tag, bool recursive);
#ifdef SWIG
}
#endif

short zusf_get_id_from_user_or_group(const std::string &user_or_group, bool is_user);
int zusf_get_file_ccsid(ZUSF *zusf, const std::string &file);
std::string zusf_get_ccsid_display_name(int ccsid);
int zusf_get_ccsid_from_display_name(const std::string &display_name);
std::string zusf_get_owner_from_uid(uid_t uid);
std::string zusf_get_group_from_gid(gid_t gid);
std::string zusf_format_ls_time(time_t mtime, bool use_csv_format = false);
std::string zusf_format_file_entry(ZUSF *zusf, const struct stat &file_stats, const std::string &file_path, const std::string &display_name, ListOptions options, bool use_csv_format, std::vector<ZusfListEntry> *entries = nullptr);

#endif
176 changes: 174 additions & 2 deletions native/python/bindings/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ The bindings are split across three modules, each generated from the correspondi
| Function | Description |
| --- | --- |
| `create_data_set(dsn: str, attributes: DS_ATTRIBUTES)` | Create a new dataset with the specified attributes. |
| `list_data_sets(dsn: str) -> list[ZDSEntry]` | List datasets matching the given pattern. |
| `list_data_sets(dsn: str, show_attributes: bool = False) -> list[ZDSEntry]` | List datasets matching the given pattern. Pass `show_attributes=True` to populate `dsorg`, `volser`, `recfm` and `migrated`. |
| `read_data_set(dsn: str, codepage: str = "") -> str` | Read content from a dataset with optional encoding. |
| `write_data_set(dsn: str, data: str, codepage: str = "", etag: str = "") -> str` | Write data to a dataset with optional encoding and etag validation. |
| `delete_data_set(dsn: str)` | Delete the specified dataset. |
Expand All @@ -23,7 +23,7 @@ The bindings are split across three modules, each generated from the correspondi
**Supporting types:**

- `DS_ATTRIBUTES`: `alcunit`, `blksize`, `dirblk`, `dsorg`, `primary`, `recfm`, `lrecl`, `dataclass`, `unit`, `dsntype`, `mgntclass`, `dsname`, `avgblk`, `secondary`, `size`, `storclass`, `vol`
- `ZDSEntry`: `name`, `dsorg`, `volser`, `recfm`, `migrated`
- `ZDSEntry`: `name`, `dsorg`, `volser`, `recfm`, `migrated` (all but `name` require `show_attributes=True`)
- `ZDSMem`: `name`

### `zjb_py` — Jobs
Expand Down Expand Up @@ -65,6 +65,178 @@ The bindings are split across three modules, each generated from the correspondi

- `ListOptions(all_files: bool = False, long_format: bool = False, max_depth: int = 1)`

## Architecture

Each module is a CPython extension. Building one runs SWIG plus two different IBM compilers, and the
finished extension has to hand strings back and forth between code that stores text as ASCII and code
that stores it as EBCDIC. This section shows where that dividing line sits, because putting it in the
wrong place corrupts your data instead of breaking the build.

### The build pipeline

`npm run z:python:build` runs `make` in this directory, and its default target is
`all: swig-extenders build`. Build the native code first with `npm run z:build` — the Metal C object
files come from the `native/c` makefile, not from this one.

```mermaid
flowchart TB
subgraph step0["Step 0 — build this first: npm run z:build (native/c makefile)"]
MCSRC["zdsm.c / zutm.c / zam.c<br/>zam24.c / zutm31.c / zutcall24.c"]
MCSRC -->|"xlc -S (Metal C, always EBCDIC)"| MCASM["build-out/*.s"]
MCASM -->|as| MCOBJ["build-out/zdsm.o, zutm.o, zam.o,<br/>zam24.o, zutm31.o, zutcall24.o"]
end

subgraph step1["Step 1 — make swig-wrappers (SWIG runs HERE, at build time only)"]
IFACE["zds_py.i<br/>%include zds_py.hpp<br/>%template ZDSEntryVector<br/>%exception to RuntimeError"]
IFACE -->|"swig -python -c++"| WRAPCXX["zds_py_wrap.cxx<br/>CPython C-API glue"]
IFACE -->|same run| PYPROXY["zds_py.py<br/>Python proxy module"]
end

subgraph step2["Step 2 — python setup.py build_ext"]
WRAPCXX --> LINK
PYPROXY -.->|"shipped as-is, never compiled"| WHEEL["py_modules"]
GLUE["zds_py.cpp<br/>hand-written glue"] --> LINK
SHARED["native/c/zds.cpp<br/>native/c/zut.cpp<br/>shared with zowex"] --> LINK
MCOBJ -->|extra_objects| LINK
LINK{{"ibm-clang++64 link"}} --> SO["_zds_py.cpython-311.so"]
end
```

**SWIG only ever runs on the build machine.** It reads the `.i` files and writes two things:
`*_py_wrap.cxx`, which gets compiled into the `.so`, and `*_py.py`, which ships exactly as generated —
that second file is the module you `import`. That is why neither distribution below needs SWIG on the
target machine. The source bundle ships the generated `.cxx` and only has to recompile it, and the
precompiled bundle ships the finished `.so` next to the `*_py.py`.

### Where the ASCII/EBCDIC line falls

The `-fzos-le-char-mode` flag tells `ibm-clang++64` which encoding to store string literals in. Left
alone the compiler picks EBCDIC, but Python's own build flags (from `sysconfig`) add
`-fzos-le-char-mode=ascii`, so every file compiled through `setup.py` gets ASCII instead. The flag
applies to one source file at a time, which lets `setup.py` choose per file:

```mermaid
flowchart LR
subgraph ASCII["ASCII literals"]
direction TB
W["zds_py_wrap.cxx<br/>SWIG-generated<br/>(CPython API needs ASCII:<br/>method names, docstrings)"]
G["zds_py.cpp<br/>+ conversion.hpp"]
end

subgraph EBCDIC["EBCDIC literals"]
direction TB
S["native/c/zds.cpp<br/>native/c/zut.cpp<br/>(CSI control blocks,<br/>TRACKS comparison)"]
end

subgraph METAL["EBCDIC, prebuilt"]
direction TB
M["build-out/*.o<br/>Metal C<br/>(no LE at all)"]
end

W <--> G
G <==>|"THE DIVIDING LINE<br/>a2e_inplace / e2a_inplace<br/>every call needs C linkage"| S
S <--> M
```

| box | where its encoding comes from |
| --- | --- |
| ASCII literals | Python's `sysconfig` build flags: `-fzos-le-char-mode=ascii` |
| EBCDIC literals | added back by `BuildExtMixedCharMode` in `setup.py`: `-fzos-le-char-mode=ebcdic` |
| EBCDIC, prebuilt | `xlc` Metal C — always EBCDIC, no flag involved |

The files under `native/c` have to stay EBCDIC. They build control blocks a byte at a time and count on
their literals already being EBCDIC: the CSI filter key in `zds_list_data_sets`, for instance, is padded
with `' '` and that blank has to be `0x40`. They also call Metal C routines, which run with no Language
Environment and know nothing about ASCII. Compiling these files as ASCII still builds cleanly — it just
quietly changes what every literal means.

### A call crossing the line

```mermaid
sequenceDiagram
participant PY as Python caller
participant PROXY as zds_py.py
participant WRAP as zds_py_wrap.cxx (ASCII)
participant GLUE as zds_py.cpp (ASCII)
participant SHARED as native/c/zds.cpp (EBCDIC)
participant MC as ZDSCSI00 Metal C (EBCDIC)

PY->>PROXY: list_data_sets("IBMUSER.**", True)
PROXY->>WRAP: _zds_py.list_data_sets(...)
WRAP->>WRAP: PyUnicode → std::string, UTF-8 bytes
WRAP->>GLUE: list_data_sets(dsn, show_attributes)
Note over GLUE,SHARED: a2e_inplace(dsn) — on the way in
GLUE->>SHARED: zds_list_data_sets, C linkage
SHARED->>MC: CSI control block, EBCDIC blank 0x40
MC-->>SHARED: catalog entries, EBCDIC
SHARED-->>GLUE: vector of ZDSEntry, EBCDIC
Note over GLUE,SHARED: e2a_inplace(name, dsorg, volser, recfm) — on the way out
GLUE-->>WRAP: vector of ZDSEntry, ASCII
WRAP-->>PROXY: ZDSEntryVector proxy
PROXY-->>PY: [ZDSEntry, ...]
Note over WRAP: a RuntimeError in Python means %exception<br/>caught a std::runtime_error thrown in zds_py.cpp
```

### Why calls across the line need `extern "C"`

IBM's libc++ gives each encoding mode its own namespace name, so an ASCII `std::string` and an EBCDIC
`std::string` end up with different symbol names in the object files. As far as the linker is concerned
they are two unrelated types. So anything the bindings call has to be declared inside an
`#ifdef SWIG extern "C"` block in the shared header — `native/c/zds.hpp`, `native/c/zjb.hpp` and
`native/c/zusf.hpp` each have one. Miss a declaration and the bind step fails with `IEW2456E`:

```mermaid
flowchart TB
subgraph broken["C++ symbol name — linker finds no match"]
direction LR
A1["zds_py.cpp, ASCII<br/>asks for std::__1_a::basic_string"] --> L1{{binder}}
B1["native/c/zds.cpp, EBCDIC<br/>offers std::__1::basic_string"] --> L1
L1 --> R1["IEW2456E SYMBOL UNRESOLVED"]
end

subgraph fixed["C linkage — linker matches it"]
direction LR
A2["zds_py.cpp, ASCII<br/>calls zds_list_data_sets"] --> L2{{binder}}
B2["native/c/zds.cpp, EBCDIC<br/>ifdef SWIG extern C block"] --> L2
L2 --> R2["matched: plain name, and<br/>std::string layout is the same"]
end

R1 ~~~ A2
```

Only the name differs; the two `std::string` types have identical memory layout. So once the linker can
find the symbol, passing a `std::string` or a `std::vector` across the line is safe. C linkage does come
with two limits:

- **It cannot handle overloads,** since every C name has to be unique. `zjb_list_by_owner` has three
overloads, so only the owner/prefix/status one is declared for SWIG; the two convenience forms sit
behind `#ifndef SWIG`.
- **It cannot return a C++ type.** `zusf_format_file_entry` returns a `std::string`, so it has to stay
outside the block.

### Which strings get converted

The rule is simple: **convert the text yourself only if the shared layer will not.** The two columns
below differ because the shared layers disagree about who converts file content.

| what crosses | `zds_py` / `zjb_py` | `zusf_py` |
| --- | --- | --- |
| DSN / path / jobid / owner / prefix / tag | `a2e` | `a2e` |
| codepage name, etag going in | `a2e` | `a2e` |
| etag coming back | `e2a` | `e2a` |
| `diag.e_msg`, listing text, `ZJob` / `ZDSEntry` fields | `e2a` | `e2a` |
| file or data set **content** | `a2e` / `e2a` | none |

`zusf` converts content in both directions on its own: it checks the requested codepage and the file
tag together, so leaving the content untouched is what makes it round-trip whether the file is tagged or
not. Data sets have no tag, so `zds` converts only when you pass an explicit codepage — which leaves the
binding to convert the content itself.

> **Known limitation:** because of that split, passing an explicit codepage to `read_data_set` or
> `write_data_set` (anything other than `""` or `"binary"`) converts the content twice and hands back an
> empty string. Etags have a matching gap: they are converted on the way out but not on the way in, so
> an etag you pass back in will never match. Stick to the default codepage until both are fixed.

## Distribution Types

### 1. Precompiled Binary Distribution (`zbind_bin_dist.tar.gz`)
Expand Down
Loading
Loading