devops: replace top-level CONTRIBUTING.md with openmoq fork stub - #157
Closed
gmarzot wants to merge 218 commits into
Closed
devops: replace top-level CONTRIBUTING.md with openmoq fork stub#157gmarzot wants to merge 218 commits into
gmarzot wants to merge 218 commits into
Conversation
Initial openmoq-main setup: - openmoq/scripts/create-release.sh (release management) - openmoq/patches/ (reserved for post-merge patches) - openmoq/README.md (branch architecture, build instructions) Workflow files (sync, publish, CI) will be added in subsequent commits.
Mirrors latest green upstream commit to main (pure mirror), then creates a merge PR from main to openmoq-main. Bot (omoq-sync-bot) creates PR, PAT (OMOQ_SYNC_TOKEN) approves, auto-merge on CI pass. No patches, no candidate branches — standard git merge handles everything.
- openmoq-ci.yml: standalone build CI for PRs to openmoq-main (ubuntu + macOS) - openmoq-publish-artifacts.yml: build + release artifacts on push to openmoq-main (4 platforms: ubuntu, macOS, bookworm-amd64, bookworm-arm64) - collect-artifacts-standalone.sh: strip and package cmake install prefix - .gitignore: add .claude/ and .vscode/
Without this, `cmake --install` only installs moxygen's own targets. Removing EXCLUDE_FROM_ALL ensures folly, fizz, wangle, mvfst, and proxygen libs+headers are also installed, producing self-contained artifact tarballs that consumers can link against directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Default OFF: EXCLUDE_FROM_ALL keeps CI builds lean (only moxygen targets). When ON: all dep libraries/headers are included in cmake --install, producing self-contained artifacts. Publish workflow uses BUNDLE_DEPS=ON and installs ICU (needed by proxygen httpserver samples). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add standalone build CI and artifact publishing
Drop BUNDLE_DEPS and ICU dependency entirely. Instead, keep EXCLUDE_FROM_ALL (avoids building unwanted proxygen samples that pull in ICU via boost::regex) and install dep libraries+headers by invoking each dep's cmake_install.cmake from the build tree. Also add sudo to container base deps for bookworm compatibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix publish-artifacts: sudo
Remove BUNDLE_DEPS option and EXCLUDE_FROM_ALL from dep add_subdirectory calls. Instead, set BUILD_SAMPLES=OFF to suppress proxygen sample targets that pull ICU via boost::regex. All dep libraries now build and install normally through cmake --install. Also add sudo to container base deps and remove ICU install step.
The publish-artifacts failures were all in the install step, which CI never exercised. Adding cmake --install to CI catches install issues before they reach publish.
Fizz install rules bake in the prefix at configure time. Using --prefix at install time doesn't override subdirectory install scripts. Set CMAKE_INSTALL_PREFIX during configure instead.
Fix publish-artifacts: BUILD_SAMPLES OFF, drop EXCLUDE_FROM_ALL
* Fix publish container paths, add test reporting Publish workflow: - Remove stale EXCLUDE_FROM_ALL dep install loop - Use relative paths for package output and artifact upload to fix container jobs where github.workspace resolves to container path but upload-artifact runs on the host CI workflow: - Add JUnit XML output from ctest (--output-junit) - Add dorny/test-reporter for test result visualization - Add checks:write permission for test-reporter * Add fault-tolerant patch system for FetchContent deps Patches in standalone/patches/<dep>/*.patch are applied between FetchContent_Populate() and add_subdirectory(). The system: - Detects already-applied patches (idempotent on cache hits) - Skips patches that no longer apply (upstream fixed the issue) - Never fails the build Cache keys now include patches directory hash so cache busts when patches are added or modified. * Fix test-reporter path: ctest writes relative to --test-dir * Speed up builds: Ninja, build types, shallow clones - Use Ninja generator for better parallelism than Make - Set CMAKE_BUILD_TYPE: RelWithDebInfo for CI, Release for publish - Add GIT_SHALLOW ON to all FetchContent declarations to skip downloading full git history (only fetch the pinned commit) * Publish separate debug tarballs alongside stripped releases Build with RelWithDebInfo to preserve debug symbols. The collect script now accepts --debug-output to create a separate unstripped tarball before stripping. Each platform produces two artifacts: moxygen-<platform>.tar.gz (stripped) moxygen-<platform>-dbg.tar.gz (with debug symbols) Both are uploaded to the GitHub Release automatically. * Bust stale cache after Ninja switch The FetchContent cache from previous runs used Unix Makefiles. Switching to Ninja causes CMake to fail when it finds a cached CMakeCache.txt with the wrong generator. Rotate cache key prefix from ci-/publish- to ci-ninja-/publish-ninja- to invalidate. * Remove GIT_SHALLOW from Meta deps that use commit SHAs GIT_SHALLOW ON passes --depth 1 to git clone, which only works with tags or branch tips. The pinned Meta dep revisions are arbitrary commit SHAs, so shallow clone fails to check them out. Keep GIT_SHALLOW ON for googletest (release-1.12.1 tag) and fast_float (v8.0.0 tag) where it works correctly. * Use split debug: extract .debug sidecars instead of full copy On Linux, use objcopy --only-keep-debug to extract debug sections into .debug sidecar files, then strip originals and add debuglinks so gdb can find them automatically. On macOS, copy unstripped libs as debug sidecars (no objcopy equivalent). The debug tarball now contains only the .debug files with matching directory structure, not a full duplicate of the install prefix. * Add email notifications, drop RelWithDebInfo from CI Email notifications via AWS SES (noreply@ci.openmoq.org): - Publish artifacts: always (success + failure) - Upstream sync: failure only Uses org-level OMOQ_AWS_ACCESS_KEY_ID/SECRET_ACCESS_KEY secrets. CI: remove -DCMAKE_BUILD_TYPE=RelWithDebInfo to avoid -O2 overhead. RelWithDebInfo adds ~9 min to Linux builds (23m vs 14m). Keep it in publish workflow where we produce release artifacts.
* Add ccache to CI and publish workflows
Uses hendrikmuhs/ccache-action@v1 for automatic ccache install,
cache save/restore, and stats. 500M cache per platform.
Cold builds remain ~17m. Warm cache builds should drop to ~2-3m
as only changed sources need recompilation.
* Add BUNDLE_DEPS option for EXCLUDE_FROM_ALL on deps
When BUNDLE_DEPS=OFF (default), deps use EXCLUDE_FROM_ALL so only
targets moxygen needs get built. Saves ~3-4 min on CI.
Publish workflow passes -DBUNDLE_DEPS=ON to build all dep targets
for artifact bundling.
* Fix container path in publish-artifacts packaging step
Relative paths (install, .) don't resolve correctly in Docker
container jobs (bookworm). Use ${{ github.workspace }} for absolute paths.
* Fix container workspace paths: use GITHUB_WORKSPACE env var
${{ github.workspace }} resolves to the host path (/home/runner/work/...)
but container jobs mount the workspace at /__w/. Using the shell env var
$GITHUB_WORKSPACE resolves correctly inside the container at runtime.
* Fix sync PR check: use GET query params instead of -f POST
- Mirror target: origin/main → origin/upstream (new branch) - Synthetic sync/<sha> PR branches (devs can push conflict fixes) - Three-phase flow: check blocking PR → advance mirror → ensure merge PR - Blocks if open sync PR exists (one at a time, notification) - Phase C always runs when unblocked (recovers from prior partial failures) - Consolidate tokens: app-token for all API calls except PR approval
- Success: ✅ + summary instead of 📦 - Failure:⚠️ instead of ❌
sync: upstream moxygen fc10745
* Soften Slack notification icons and add brief summaries - Success: ✅ + summary instead of 📦 - Failure:⚠️ instead of ❌ * Fix auto-merge: use PAT instead of app token The GitHub App token can't call enablePullRequestAutoMerge (GraphQL mutation requires a user token). Switch to OMOQ_SYNC_TOKEN.
* Fix moxygen headers missing from artifact tarballs - Add moxygen_install_headers() to standalone CMakeLists.txt (was only in the top-level build, not the standalone build) - Fix collect-artifacts fallback: use relative paths to avoid leaking CI workspace paths (__w) into tarball, check for actual .h files not just directory existence * Fix cmake config install paths: each dep gets its own directory All Meta deps share a CMAKE_INSTALL_DIR cache variable. Folly sets it first to lib/cmake/folly, and fizz/wangle/proxygen silently inherit it. Force-set before each add_subdirectory so configs install to lib/cmake/<dep>/ where find_package() can find them. Moxygen install section uses a local variable to avoid the shared cache.
* Updating hashes Summary: GitHub commits: facebook/fb303@caeee07 facebook/fbthrift@1fcbcb0 facebook/folly@93187f7 facebook/mvfst@9f47ad6 facebook/proxygen@8d14f87 facebook/wangle@0cb4f4c facebookexperimental/edencommon@fb82a3b facebookexperimental/rust-shed@234c074 facebookincubator/fizz@d2fed4c Reviewed By: sdwilsh fbshipit-source-id: c7f4eac046274b1017fd080d719af864e728e17c * Add auto-merge-sync workflow: merge sync PRs via workflow_run on CI pass Replaces the unreliable enablePullRequestAutoMerge GraphQL call in the upstream-sync workflow. Fires when OpenMOQ CI completes successfully on a sync/* branch and merges the corresponding open PR via REST API. --------- Co-authored-by: Open Source Bot <generatedunixname499836121@meta.com> Co-authored-by: Giovanni Marzot <gmarzot@marzresearch.net>
* Retry asset uploads in create-release.sh: split create from upload Large file uploads to GitHub releases are prone to transient 502/404 errors. Split gh release create (metadata only) from per-asset upload, with 3 attempts and exponential backoff. --clobber makes retries idempotent. * Fix publish notifications: check job.status for release step failures Notifications only checked needs.build.result, so a successful build with a failed release upload still sent a success alert. Now also gate on job.status so any step failure in the release job fires correctly. * Remove header supplement fallback from collect-artifacts-standalone.sh moxygen_install_headers() now handles header installation via cmake. The fallback supplementation from source is no longer needed. Remove --src-dir option entirely. * Restrict auto-merge trigger to sync/* branches only
sync: upstream moxygen 8ab128a
…39) Remove the 'Enable auto-merge' step from upstream-sync.yml — it used gh pr merge --auto which calls the enablePullRequestAutoMerge GraphQL mutation. This requires 'Allow auto-merge' in repo settings and is redundant since openmoq-auto-merge-sync.yml already merges via REST after CI passes. Add notify-ci-failure job to auto-merge-sync.yml so conflicts on sync branches trigger a Slack alert instead of failing silently.
sync: upstream moxygen aaa0011
sync: upstream moxygen 338aa49
sync: upstream moxygen 6cd12c8
* CI: add ASAN build job, rename openmoq-main to main in workflows * workflows: rename to lowercase, amd64 job name, updated Slack format - openmoq-ci.yml → pull request ci; asan debug job; test (linux/macos/asan debug) reporters - openmoq-publish-artifacts.yml → release publish; ubuntu-22.04-amd64 job name; structured Slack notification - openmoq-upstream-sync.yml → upstream sync - openmoq-auto-merge-sync.yml → sync auto-merge; update workflow_run trigger to match new CI name * ci: simplify job names to linux / macos / asan debug * ci: merge asan debug into build matrix
…or parity with moqx (#174)
sync: upstream moxygen 399001f
sync: upstream moxygen 187c140
sync: upstream moxygen 6df06a3
Upstream introduced getOrCreateGroupWithEviction and new addGroupToLRU call sites that pass 3 args, but our local main carries a 4-arg addGroupToLRU(ftn, groupID, group, track) for global LRU bookkeeping. The textual merge was clean; the semantic conflict was not. Add FullTrackName to getOrCreateGroupWithEviction and forward it to addGroupToLRU so all call sites compile against the 4-arg signature.
sync: upstream moxygen 7873433
…ggable Linux RelWithDebInfo entries (#184)
sync: upstream moxygen a862789
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Alan Frindell <8259689+afrind@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
deps: bump openmoq/picoquic pin to 71468949a95a
sync: upstream moxygen eae0206
sync: openmoq/picoquic pin to afe4309d0dd9
Replace upstream Meta's CONTRIBUTING.md at top-level with a brief stub that identifies this as a fork and points contributors to: - .github/CONTRIBUTING.md for fork-specific contribution guidance (already canonical — GitHub UI prefers .github/ when present) - upstream's CONTRIBUTING.md (via URL) for Meta repo contributions Approach (vs. a full overwrite): - Stub is ~140 lines smaller than the full openmoq doc - No content duplication between top-level and .github/ - URL link to upstream stays current automatically; no copied content to go stale Carry-patch survival: the stub diverges completely from upstream's CONTRIBUTING.md, so omoq-upstream-sync's `-X ours` keeps it on every future sync.
meta-cla
Bot
added
the
CLA Signed
Contributor
Author
|
Closing — opened against the wrong repo by mistake (gh CLI defaulted to upstream parent because of a local upstream remote). The intended target is openmoq/moxygen, our fork. Reposting there. Apologies for the noise. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #167.
Summary
Replace upstream Meta's CONTRIBUTING.md at top-level with a brief stub that identifies this as a fork and points contributors to the right place. Closes the "developers see Meta CLA process and think it applies here" confusion that #167 raised.
Diff
CONTRIBUTING.md: Meta's 35-line upstream doc → 6-line openmoq fork stub..github/CONTRIBUTING.md: untouched (canonical fork doc lives here, GitHub UI prefers it).Why a stub instead of full overwrite
.github/. Single source of truth in.github/.cat CONTRIBUTING.mdfrom a clone now sees "this is openmoq's fork — see .github/CONTRIBUTING.md or upstream's" instead of Meta's CLA process. GitHub PR-flow contributors were already seeing.github/CONTRIBUTING.mdbecause GH UI prefers it; this just closes the terminal-cat hole.Carry-patch survival
The stub diverges completely from upstream's CONTRIBUTING.md, so any future upstream change creates a hunk conflict and
omoq-upstream-sync.yml'sgit merge -X oursresolution keeps our stub. Upstream changes are silently swallowed at the merge level — but that's fine for this carry-patch because the stub doesn't copy content, only links to upstream by URL. The link remains correct as upstream evolves.Test plan
.github/CONTRIBUTING.mdrenders correctly in GitHub's "Contributing guidelines" UI link (community profile / PR template) — should still surface openmoq's full doc as before.CONTRIBUTING.mdrenders cleanly in repo root view.[.github/CONTRIBUTING.md](.github/CONTRIBUTING.md)and the absolute upstream URL.