update patches#26
Conversation
|
augment review |
🤖 Augment PR SummarySummary: Updates the port to zstd Changes:
Technical Notes: The patch set now relies more on upstream’s 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| #include <stdio.h> /* fprintf, fopen */ | ||
| #include <stdlib.h> /* malloc, free */ | ||
| #include <string.h> /* memset, strerror */ | ||
| +#include "platform.h" /* Large Files support, compiler specifics */ |
There was a problem hiding this comment.
platform.h is documented upstream as needing to be included first (e.g., for large-file/feature macros), but this patch moves it (and in one case util.h) after system headers in several translation units, which can cause subtle ABI/IO issues. Consider keeping these headers first (or ensuring the required macros are provided via compiler flags) to preserve the intended behavior.
Severity: medium
Other Locations
patches/assertfirst.patch:36patches/assertfirst.patch:58patches/assertfirst.patch:77patches/assertfirst.patch:95
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
- Disable unsupported -soname binder option - Add -shared to CFLAGS for dynamic library objects - Link with -lzoslib -lstdc++ to resolve missing symbols - Reorder includes in program source to fix assert macro conflict - Update buildenv to use git repo and minimal config
|
augment review |
| export ZOPEN_STABLE_DEPS="curl gzip make coreutils zoslib check_python" | ||
| export ZOPEN_STABLE_URL="https://github.com/facebook/zstd.git" | ||
| export ZOPEN_STABLE_TAG="v${ZSTD_VERSION}" | ||
| export ZOPEN_STABLE_DEPS="make coreutils check_python diffutils which" |
There was a problem hiding this comment.
buildenv:8 ZOPEN_STABLE_URL is now a git repo, but ZOPEN_STABLE_DEPS no longer includes any tool that guarantees git is available; this may cause source-fetch failures in minimal build environments.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| zopen_get_version() | ||
| { | ||
| ./zstd --version | head -1 | awk '{print $5; }' | tr -d ',v' | ||
| echo $ZSTD_VERSION |
There was a problem hiding this comment.
buildenv:52 zopen_get_version() now returns the configured $ZSTD_VERSION instead of querying the built ./zstd, which can hide version/build mismatches (e.g., if the checked-out tag differs from what was built/installed).
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| $(LIBZSTD): CPPFLAGS += $(CPPFLAGS_DYNLIB) | ||
| $(LIBZSTD): CFLAGS += -fPIC -fvisibility=hidden | ||
| +ifeq ($(UNAME_TARGET_SYSTEM), OS/390) | ||
| +$(LIBZSTD): CFLAGS += -shared |
There was a problem hiding this comment.
patches/lib.patch:32 Adding -shared to CFLAGS means it may be used during compilation steps as well as linking; if the toolchain treats -shared as link-only, this can fail or produce unexpected build behavior.
Severity: medium
Other Locations
patches/lib.patch:60
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
/run tests |
1 participant
No description provided.