Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 16 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,22 @@ NVBIT_PATH=./third_party/nvbit/core
INCLUDES=-I$(NVBIT_PATH) -I./$(INCLUDE_DIR) -I./third_party

# Libraries
LIBS=-L$(NVBIT_PATH) -lnvbit -lzstd
# Dynamically find static zstd library for self-contained builds
# Priority: 1) pkg-config, 2) common system paths
# Note: -lpthread is required because zstd uses POSIX threads internally
ZSTD_STATIC := $(shell pkg-config --variable=libdir libzstd 2>/dev/null)/libzstd.a
ifeq ($(wildcard $(ZSTD_STATIC)),)
ZSTD_STATIC := $(shell for p in /usr/lib64 /usr/lib /usr/local/lib64 /usr/local/lib; do \
if [ -f "$$p/libzstd.a" ]; then echo "$$p/libzstd.a"; break; fi; done)
endif
ifeq ($(ZSTD_STATIC),)
$(warning WARNING: libzstd.a not found - falling back to dynamic linking.)
$(warning WARNING: The resulting binary will NOT be self-contained/portable.)
$(warning WARNING: Install static library with: dnf install libzstd-static (RHEL/Fedora) or apt install libzstd-dev (Ubuntu/Debian))
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.
LIBS=-L$(NVBIT_PATH) -lnvbit $(ZSTD_STATIC) -lpthread
NVCC_PATH=-L $(subst bin/nvcc,lib64,$(shell which nvcc | tr -s /))

# Identify inject_funcs.cu specifically
Expand Down
13 changes: 10 additions & 3 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,21 @@ git clone [email protected]:facebookresearch/CUTracer.git
cd CUTracer
```

2. Install system dependencies (libzstd):
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.
# On most Ubuntu/Debian systems, libzstd-dev provides both shared and static libs (libzstd.a).
# You can verify this with: 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.
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.

# CentOS/RHEL
sudo dnf install libzstd-devel
# CentOS/RHEL/Fedora (static library for portable builds)
sudo dnf install libzstd-static

# If static library is not available, the build will fall back to dynamic linking
# and display a warning. The resulting binary will not be self-contained.
```

3. Download third-party dependencies:
Expand Down