Skip to content

Conversation

@FindHao
Copy link
Contributor

@FindHao FindHao commented Dec 22, 2025

Summary

This PR modifies the build system to statically link libzstd, making cutracer.so self-contained and portable across different environments where the dynamic libzstd may not be available.

Changes

  • Makefile: Dynamically search for libzstd.a in common system paths (/usr/lib64, /usr/lib, /usr/local/lib64, /usr/local/lib) with pkg-config fallback
  • Makefile: Improved warning messages when static library is not found - now explicitly states that the build will fall back to dynamic linking and the resulting binary will NOT be self-contained/portable
  • Makefile: Added -lpthread to link flags (required because zstd uses POSIX threads internally for multi-threaded compression)
  • README: Updated installation instructions with verification steps for Ubuntu/Debian users to confirm libzstd.a is available

Why

When using CUTracer via CUDA_INJECTION64_PATH in certain environments, the dynamic libzstd.so.1 may not be available in the library search path, causing library load failures. Static linking resolves this dependency issue by embedding zstd into cutracer.so.

Test Plan

  1. Install static zstd library:
    • RHEL/Fedora: sudo dnf install libzstd-static
    • Ubuntu/Debian: sudo apt install libzstd-dev (verify with dpkg -L libzstd-dev | grep libzstd.a)
  2. Rebuild CUTracer: make clean && make
  3. Verify no dynamic zstd dependency: ldd lib/cutracer.so | grep zstd (should be empty)
  4. Test with CUDA_INJECTION64_PATH in target environment

Copilot AI review requested due to automatic review settings December 22, 2025 19:47
@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Dec 22, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modifies the build system to statically link libzstd instead of using dynamic linking, making cutracer.so self-contained and portable for environments where the dynamic libzstd may not be available (e.g., Meta's Buck PAR).

Key Changes:

  • Implements static libzstd detection in the Makefile with pkg-config and system path fallback
  • Updates README installation instructions to recommend static library packages
  • Adds -lpthread dependency to the linker flags

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
Makefile Adds logic to dynamically locate and link static libzstd with fallback to dynamic linking; adds -lpthread to linker flags
readme.md Updates installation instructions to mention static library packages for portable builds

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


```bash
# Ubuntu/Debian
sudo apt-get install libzstd-dev
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The CI setup script still installs libzstd-dev which only includes the dynamic library on Ubuntu/Debian. For consistency with the PR's goal of creating self-contained builds, consider updating this to also install the static library or documenting that CI builds may use dynamic linking.

Copilot uses AI. Check for mistakes.
2. Install system dependencies (libzstd static library for self-contained builds):

```bash
# Ubuntu/Debian
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The installation instructions for Ubuntu/Debian show libzstd-dev, but this package typically only includes the dynamic library. To achieve the self-contained builds mentioned in the PR goal, Ubuntu/Debian users may need additional steps or a different package. Consider clarifying whether libzstd-dev on Ubuntu/Debian includes the static library, or provide explicit instructions for obtaining it (e.g., building from source or a specific package name).

Suggested change
# Ubuntu/Debian
# Ubuntu/Debian
# On most Ubuntu/Debian systems, libzstd-dev provides both shared and static libs (libzstd.a).
# You can verify this with, e.g.,: dpkg -L libzstd-dev | grep 'libzstd.a'
# If your distribution does not ship the static library in libzstd-dev, you may need to
# build zstd from source or install a distro-specific static libzstd package.

Copilot uses AI. Check for mistakes.
$(warning WARNING: libzstd.a not found. Install with: dnf install libzstd-static)
ZSTD_STATIC := -lzstd
endif

Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The -lpthread flag has been added here but it's not mentioned in the PR description or changes summary. If this is an unrelated fix or dependency, it should either be documented in the PR description or removed from this PR and addressed separately to keep changes focused.

Suggested change
# Note: -lpthread is required because the tool/library uses POSIX threads (e.g., std::thread).

Copilot uses AI. Check for mistakes.
Makefile Outdated
Comment on lines 57 to 58
$(warning WARNING: libzstd.a not found. Install with: dnf install libzstd-static)
ZSTD_STATIC := -lzstd
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

When the static library is not found, the code falls back to -lzstd which will use dynamic linking. This means the build will succeed but won't achieve the stated goal of creating a self-contained binary. Consider making this a hard error instead of a warning, or at least make the warning message more explicit that the build will fall back to dynamic linking and won't be portable.

Suggested change
$(warning WARNING: libzstd.a not found. Install with: dnf install libzstd-static)
ZSTD_STATIC := -lzstd
$(error ERROR: libzstd.a not found. Install with: dnf install libzstd-static. \
Static libzstd.a is required to produce a self-contained binary; falling back to dynamic libzstd.so is not allowed.)

Copilot uses AI. Check for mistakes.
- Dynamically search for libzstd.a in common system paths
- Improved warning messages when static library not found - explicitly states
  that build will fall back to dynamic linking and result won't be portable
- Added -lpthread to link flags (required by zstd's internal threading)
- Updated README with verification steps for Ubuntu/Debian users
@FindHao FindHao force-pushed the findhao/static-zstd-linking branch from 7361543 to f86d376 Compare December 22, 2025 20:04
@meta-codesync
Copy link

meta-codesync bot commented Dec 22, 2025

@FindHao has imported this pull request. If you are a Meta employee, you can view this in D89684854.

@meta-codesync
Copy link

meta-codesync bot commented Dec 23, 2025

@FindHao merged this pull request in 837eab6.

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

Labels

CLA Signed This label is managed by the Meta Open Source bot. Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants