Skip to content

DOCS: Add third-party notices - #12071

Merged
gleon99 merged 5 commits into
openucx:masterfrom
roiedanino:docs/third-party-notices
Oct 21, 2026
Merged

gleon99 merged 5 commits into
openucx:masterfrom
roiedanino:docs/third-party-notices

Conversation

@roiedanino

@roiedanino roiedanino commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What?

Add a top-level THIRD_PARTY_NOTICES file covering copyright holders for UCX
sources distributed under the project BSD license, separately licensed source
bundled with UCX, and the pinned GPUNetIO submodule. Link the notice from the
README and include it in source, RPM, and Debian packages.

Why?

Make license and attribution obligations explicit and ensure the notices
accompany UCX distributions.

How?

Inventory the copyright holders in project-BSD source files and the separately
licensed bundled components, preserve the applicable license and attribution
text, and record the same inventory in the Debian copyright metadata.
Separately installed libraries and tools remain outside the notice scope.

@svc-nvidia-pr-review

Copy link
Copy Markdown

🤖 Starting review — findings will be posted here when done.

Comment thread THIRD_PARTY_NOTICES Resolved
Comment thread THIRD_PARTY_NOTICES Resolved
Comment thread debian/ucx.docs Resolved
@svc-nvidia-pr-review

Copy link
Copy Markdown

🤖 Starting review — findings will be posted here when done.

@svc-nvidia-pr-review

Copy link
Copy Markdown

🤖 Starting review — findings will be posted here when done.

@roiedanino
roiedanino requested a review from gleon99 October 12, 2026 08:03
@roiedanino roiedanino self-assigned this Oct 12, 2026
@svc-ucx

svc-ucx commented Oct 12, 2026

Copy link
Copy Markdown

🤖 CI Triage Agent — UCX PR (Tests roce on worker 3) · commit 5b819790

TL;DR: The gtest shm_ib/test_ucp_wireup_1sided.stress_connect2/2 <shm,ib/rma,no_ep_match> hung on the RoCE RC/mlx5 RMA lane — PUT payload was never delivered even though ucp_ep_flush_nbx returned, and the subsequent ucp_ep_close_nbx request never completed, so the 900 s gtest watchdog aborted the process (SIGABRT). This is an infrastructure/master-side RMA completion bug, not caused by PR #12071 (a docs-only branch); retrigger the job and investigate the recent mlx5 purge-outstanding/completion changes.

Full analysis

Summary: test_ucp_wireup_1sided.stress_connect2/2 (variant shm,ib/rma,no_ep_match) stalled in ucp_test::request_process during disconnect(), tripped the 15-minute watchdog, and make test died with "Aborted (core dumped)".

Root cause: A hang, not slowness. The test entered stress_connect2/2 at 10:06:20 and produced no output for 3m17s until 10:09:37 ... request 0x55940ab4a158 did not complete on time — i.e. RMA requests sat unfinished for the full 180 s request deadline, repeatedly (10:09:37, 10:12:37, 10:15:47, 10:18:48). In between, test_ucp_wireup.cc:361 reported silent data loss: recv_data = 54735 but data = 0 at index 0. That is diagnostic — recv_b() only reaches line 361 after wait_for_value(&m_recv_data[length], recv_data) observes the trailing control PUT (test_ucp_wireup.cc:304-308), so the control word landed while the preceding payload PUT (:287) did not, despite the intervening ucp_ep_flush_nbx at :300-302 having completed. A flush that reports completion while its PUT data is still undelivered means a UCT send completion was accounted for but never actually delivered on the wire on the RC/mlx5 RoCE lane. The same lost-completion condition then wedged EP teardown: the backtrace shows the final hang in ucp_ep_close_nbx → request_wait (test_ucp_wireup.cc:408, reached from :412/:639), which spun in ucp_test::progress until test_helpers.cc:57 fired "Connection timed out - abort testing" at exactly 900 s after test start (watchdog_timeout = 900., test_helpers.cc:31) and raised signal 6. Note src/ucp/rma/rma.h:18-27 already documents a known multi-lane flush/RMA-ordering bug (hence UCP_PROTO_RMA_MAX_BCOPY_LANES 1) and test_ucp_wireup.cc:295-298 carries a FIXME about flush-vs-fence in this exact code path. PR #12071 is on branch docs/third-party-notices and touches no UCP/UCT source, so it cannot have introduced this.

Implicated commit: Not the PR under test. Most likely candidates are the recent mlx5 send-completion/purge-outstanding series landing days before this 2026-10-12 build: 757f9f9b "UCT/IB/MLX5: support rma get in purge outstanding (#11866)" (Zihao Zhao, 2026-10-09), 2dbbd8c4 "UCT/API: Purge outstanding work without an RX token (#12006)" (Evgeny Leksikov, 2026-10-04), 7f839dd3 "UCT/IB/MLX5: support put zcopy in purge outstanding (#11874)", and 68cfcbc5 "UCT/IB/MLX5: uct comp marker (#11915)" — all touch src/uct/ib/mlx5/rc/rc_mlx5_ep.c completion accounting. Unconfirmed from this log alone.

File: test/gtest/ucp/test_ucp_wireup.cc:361 (assertion) and :408 (hang site); suspect product code src/uct/ib/mlx5/rc/rc_mlx5_ep.c completion/purge path; related known-bug note at src/ucp/rma/rma.h:18-27

Suggested fix: Do not raise the job time limit — the 900 s kill is the watchdog catching a hang. (1) Re-run the roce worker-3 job to confirm this is flaky/unrelated to the docs PR; since #12071 changes only documentation, it should be merged on a green retry. (2) For the real bug, reproduce with UCX_TLS=shm,ib ./gtest --gtest_filter='*test_ucp_wireup_1sided.stress_connect2*rma*' in a loop with UCX_LOG_LEVEL=trace_req and IB trace-data enabled, and verify RC/mlx5 send-completion accounting: specifically that uct_ep_flush cannot signal completion before all posted PUT WQEs are CQE-acked, and that the purge-outstanding paths added in #11836/#11866/#11874/#12006 do not consume or re-signal a completion for a still-unposted/undelivered PUT. (3) Add an assertion or debug check that a completed ucp_ep_flush_nbx implies zero outstanding PUTs on every lane in the lane_map, which would turn this silent data-loss-then-hang into an immediate, localizable failure.

Related: PR #11866, PR #11874, PR #11836, PR #12006, PR #12019 (mlx5 purge-outstanding series); PR #11915 (uct comp marker); PR #12002 / PR #12015 (recent put_offload.c multi-rail RMA changes); PR #11873 (still open in the same series)

@roiedanino

Copy link
Copy Markdown
Contributor Author

@gleon99 @yosefe

Comment thread debian/copyright Resolved
Comment thread THIRD_PARTY_NOTICES Resolved
@svc-nvidia-pr-review

Copy link
Copy Markdown

🤖 Starting review — findings will be posted here when done.

@svc-ucx

svc-ucx commented Oct 14, 2026

Copy link
Copy Markdown

🤖 CI Triage Agent — UCX PR (Build build_aarch64 sles15sp7_aarch64) · commit dc5de3c2

TL;DR: The aarch64 SLES RPM build fails with error: Installed (but unpackaged) file(s) found: /usr/share/doc/ucx/{LICENSE,THIRD_PARTY_NOTICES} because this PR's new dist_doc_DATA rule in Makefile.am installs those files into autotools' docdir (/usr/share/doc/ucx), which no %files section in ucx.spec.in claims. Fix by either claiming that path in the spec or removing it in %install (RPM already ships the files via %license).

Full analysis

Summary: build_aarch64 / sles15sp7_aarch64 failed during rpmbuild (contrib/buildrpm.sh) at the check-files step — two newly installed doc files are not owned by any sub-package.

Root cause: Commit dc5de3c2 ("DOCS: Install legal notices with UCX") added to Makefile.am:

dist_doc_DATA = \
	LICENSE \
	THIRD_PARTY_NOTICES

Automake's docdir defaults to $(datarootdir)/doc/$(PACKAGE) = /usr/share/doc/ucx, and the log confirms make install placing them there:

/usr/bin/mkdir -p '.../_rpm/usr/share/doc/ucx'
/usr/bin/install -c -m 644 LICENSE THIRD_PARTY_NOTICES '.../_rpm/usr/share/doc/ucx'

Meanwhile ucx.spec.in only declares %doc README AUTHORS NEWS and %license LICENSE THIRD_PARTY_NOTICES. On SUSE, RPM's %_docdir is /usr/share/doc/packages and %_licensedir is /usr/share/licenses, so the %doc/%license directives copied the files again from the source tree into /usr/share/doc/packages/ucx and /usr/share/licenses/ucx (both visible in the log) — leaving the autotools-installed copies under /usr/share/doc/ucx unowned. /usr/share/doc/ucx is also not covered by the existing %{_datadir}/ucx glob. rpmbuild's check-files then aborts, and the Bash task exits 1.

Note this is latent on Fedora/RHEX too whenever %_docdir != /usr/share/doc or the installed path isn't globbed; the Debian build passes only because debian/ucx.install has a catch-all usr/share/*.

Implicated commit: [REDACTED:Hex High Entropy String] — Roie Danino, "DOCS: Install legal notices with UCX" (builds on 48121a1b, same author, "DOCS: Add third-party notices")

File: Makefile.am:24-26 (the dist_doc_DATA block) interacting with ucx.spec.in:163-165

Suggested fix: Pick one of these:

  1. Preferred — make the spec own the installed path instead of re-copying from the source tree. In ucx.spec.in, replace line 165 with a reference to the actually-installed location so it works regardless of distro %_docdir:

    %license %{_datadir}/doc/%{name}/LICENSE
    %license %{_datadir}/doc/%{name}/THIRD_PARTY_NOTICES
    %dir %{_datadir}/doc/%{name}

    (keeping %doc README AUTHORS NEWS as-is).

  2. Simplest — let RPM keep its own copies and drop the duplicate. Add to %install in ucx.spec.in (after line 149):

    rm -rf %{buildroot}%{_datadir}/doc/%{name}

    This leaves %license LICENSE THIRD_PARTY_NOTICES as the single source of truth and preserves distro-correct %_licensedir placement.

  3. Alternative — align docdir with RPM. Pass --docdir=%{_docdir}/%{name} in the %configure invocation so automake installs where %doc expects.

Whichever route is taken, re-run the release / RPM-building legs (not just the Debian one) since debian/ucx.install's usr/share/* glob masks this class of bug.

Related: PR #12071 (this change); prior commit 48121a1b "DOCS: Add third-party notices" — no existing issue found for this error signature.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 37de85a4-24cc-433e-a64f-d4f02888182d in the triage console for the audit trail.

Comment thread debian/copyright
2001-2016 NVIDIA CORPORATION & AFFILIATES. ALL RIGHTS RESERVED.
License: BSD-3-clause and BSD-3-clause-UC

Files: test/gtest/common/googletest/*

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This stanza also captures Makefile.am and configure.m4 in that directory, which are UCX files (Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2021 / NextSilicon Ltd. 2021), not Google's. In DEP-5 the later stanza wins over Files: *, so those two get attributed to Google 2005-2015 under BSD-3-clause-Google. Please add a follow-up stanza putting them back under BSD-3-clause.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: added specific BSD-3-Clause stanzas after the GoogleTest wildcard for Makefile.am and configure.m4.

Comment thread debian/copyright Outdated
License: Apache-2.0 or BSD-3-clause
Comment: UCX redistributes this file under the BSD-3-clause alternative.

Files: external/gpunetio/*

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

external/gpunetio is a submodule path, and the GPUNetIO headers are shipped from src/uct/ib/mlx5/gdaki/gpunetio/ (see nobase_dist_libuct_ib_mlx5_gda_la_HEADERS), so this stanza matches nothing in the release tarball the Debian package is built from. Please cover the in-tarball path as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: the DEP-5 stanza now covers both the submodule path and the in-tarball src/uct/ib/mlx5/gdaki/gpunetio path.

Comment thread Makefile.am
doxygen_doc_files = $(noinst_HEADERS)

doc_dir = $(pkgdatadir)/doc
dist_doc_DATA = \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This installs LICENSE and THIRD_PARTY_NOTICES into $(docdir) (/usr/share/doc/ucx), but %files in ucx.spec.in only lists %{_datadir}/ucx, %doc README AUTHORS NEWS, and %license. Did contrib/buildrpm.sh pass with the new paths? rpm aborts on unpackaged files, and build_release_pkg in buildlib/tools/builds.sh runs it. (Caveat: rpm's special %doc handling may emit /usr/share/doc/ucx as a bare recursive directory entry, which would silently package the new files — and ship LICENSE twice, once under %{_licensedir} and once under docdir — rather than fail, hence a question rather than a blocker.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: RPM packaging removes the duplicate Automake docdir copy and keeps the distro-native %license copies. A source and binary RPM build passed, both legal files are present under the RPM license directory, and /usr/share/doc/ucx is absent from the buildroot.

Comment thread THIRD_PARTY_NOTICES Outdated

Doxygen Autoconf macros

Files:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor: docs/doxygen/doxygen.am is listed as FSFAP only here, but debian/copyright L79-83 says BSD-3-clause and FSFAP for the same file (its header has both See file LICENSE for terms and the FSFAP notice). Please align the two files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: split the two Doxygen files in THIRD_PARTY_NOTICES and marked docs/doxygen/doxygen.am as BSD-3-Clause and FSF All Permissive License.

@svc-nvidia-pr-review

Copy link
Copy Markdown

Residual coverage gap: the only validation for these packaging changes is the build_release_pkg job (make distcheck → buildrpm.sh / dpkg-buildpackage). Since dist_doc_DATA adds installed files at a path no %files entry mentions, CI results for that job would be the evidence to confirm before merge.

@svc-nvidia-pr-review

Copy link
Copy Markdown

🤖 Starting review — findings will be posted here when done.

Comment thread ucx.spec.in
rm -f %{buildroot}%{_libdir}/libucs_signal.a
rm -f %{buildroot}%{_libdir}/ucx/*.la
rm -f %{buildroot}%{_libdir}/ucx/lib*.so
rm -rf %{buildroot}%{_datadir}/doc/%{name}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hardcoded %{_datadir}/doc does not match %{_docdir} on all distros.

Suggested change
rm -rf %{buildroot}%{_datadir}/doc/%{name}
rm -rf %{buildroot}%{_docdir}/%{name}

dist_doc_DATA installs to automake's docdir, which is $(datarootdir)/doc/$(PACKAGE_TARNAME) unless --docdir is passed. The RPM %configure macro is distro-patched: on Fedora/RHEL %{_docdir} is /usr/share/doc (so the new rm works), but on SUSE %{_docdir} is /usr/share/doc/packages and %configure passes --docdir. There the files land outside %{_datadir}/doc/%{name}, survive the rm, and rpmbuild fails with "Installed (but unpackaged) file(s) found". %{_docdir}/%{name} is correct in both cases since %{_docdir} expands to %{_datadir}/doc on Fedora. The spec already has SUSE-specific conditionals, so SUSE is in scope.

Comment thread THIRD_PARTY_NOTICES
src/ucs/algorithm/qsort_r.c
src/ucs/algorithm/qsort_r.h
Copyright (c) 1992, 1993 The Regents of the University of California.
License: BSD-3-Clause

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The qsort_r stanza understates the file's licensing: qsort_r.c/.h also carry Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2001-2016 and See file LICENSE for terms, and the reproduced text is the clause-4 UC variant, not plain BSD-3-Clause. debian/copyright in this PR says BSD-3-clause and BSD-3-clause-UC with both copyrights listed; pls make this entry match so the two new files don't disagree.

Comment thread THIRD_PARTY_NOTICES
----------------------

GoogleTest
Files: test/gtest/common/googletest/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blanket GoogleTest directory claim over-attributes to Google: googletest/Makefile.am (NVIDIA + NextSilicon) and googletest/configure.m4 (NVIDIA) are UCX-authored. debian/copyright carves them out in separate stanzas, but this blanket Files: test/gtest/common/googletest/ claim attributes them to Google.

Comment thread THIRD_PARTY_NOTICES
UCX redistributes this file under the BSD-3-Clause alternative.

NVIDIA DOCA GPUNetIO
Git submodule: external/gpunetio

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The gpunetio entry omits the in-tree path: pls also list src/uct/ib/mlx5/gdaki/gpunetio/* here; that is the symlinked path whose headers src/uct/ib/mlx5/gdaki/Makefile.am actually installs, and debian/copyright already covers both external/gpunetio/* and that path.

Comment thread THIRD_PARTY_NOTICES
SGLIB 1.0.3
File: src/ucs/datastruct/sglib.h
Upstream: http://www.xref-tech.com/sglib
Copyright (C) 2003-2005 Marian Vittek

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

minor: the SGLIB entry is missing the License: field — pls add License: SGLIB; every other component entry has an explicit License: line, and debian/copyright uses that short name.

Comment thread THIRD_PARTY_NOTICES
-----------------------

UCX project sources
Files: UCX source files that refer to the top-level LICENSE

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LICENSE still lists only the 2014-2020 holders while this file and debian/copyright now carry the full list (~21 entries) through 2026. Both new files point at LICENSE as authoritative — should LICENSE be updated too, or is leaving it as-is intentional?

@roiedanino
roiedanino requested a review from gleon99 October 20, 2026 11:58
@gleon99
gleon99 merged commit 6aa2cf1 into openucx:master Oct 21, 2026
165 checks passed
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.

4 participants