diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000000000..84f442c0fae07 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,111 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project Overview + +Sapling SCM is a cross-platform, highly scalable, Git-compatible source control system. It consists of three main components: + +- **Sapling CLI** (`eden/scm/`): The `sl` command-line tool, based on Mercurial. Written in Python and Rust. +- **Interactive Smartlog (ISL)** (`addons/`): Web-based and VS Code UI for repository visualization. React/TypeScript. +- **Mononoke** (`eden/mononoke/`): Server-side distributed source control server (Rust). Not yet publicly supported. +- **EdenFS** (`eden/fs/`): Virtual filesystem for large checkouts (C++). Not yet publicly supported. + +## Build Commands + +### Sapling CLI (primary development) + +From `eden/scm/`: + +```bash +make oss # Build OSS version for local usage (creates ./sl binary) +make install-oss # Install CLI to $PREFIX/bin +make local # Build for inplace usage +make clean # Remove build artifacts +``` + +### Running Tests + +```bash +# All tests (from eden/scm/) +make tests + +# Single test +make test-foo # Runs test-foo.t + +# Using getdeps (full CI-style testing) +./build/fbcode_builder/getdeps.py test --allow-system-packages --src-dir=. sapling + +# Single test with getdeps +./build/fbcode_builder/getdeps.py test --allow-system-packages --src-dir=. sapling --retry 0 --filter test-foo.t +``` + +### ISL (Interactive Smartlog) + +From `addons/`: + +```bash +yarn install # Install dependencies +yarn dev # Start dev server for ISL +``` + +From `addons/isl/`: + +```bash +yarn start # Start Vite dev server +yarn build # Production build +yarn test # Run Jest tests +yarn eslint # Lint TypeScript/React code +``` + +### Website + +From `website/`: + +```bash +yarn start # Start Docusaurus dev server +yarn build # Build static site +yarn lint # Run ESLint and Stylelint +yarn format:diff # Check formatting with Prettier +yarn typecheck # TypeScript type checking +``` + +## Architecture + +### Sapling CLI (`eden/scm/`) + +- `sapling/` - Python CLI modules and commands +- `lib/` - Rust libraries (117+ modules): dag, indexedlog, gitcompat, checkout, clone, etc. +- `exec/` - Rust executables: hgmain, scm_daemon +- `tests/` - `.t` test files (1000+) + +### ISL (`addons/`) + +Yarn workspace with: +- `isl/` - React/Vite web UI +- `isl-server/` - Node.js backend +- `vscode/` - VS Code extension +- `shared/` - Shared utilities +- `components/` - Reusable UI components + +## Code Style + +- 2 spaces indentation +- 80 character line length +- Rust: 2024 edition, rustfmt configured in `.rustfmt.toml` +- TypeScript/JavaScript: ESLint + Prettier +- Python: Flake8 + +## Dependencies + +- Python 3.8+ +- Rust (stable) +- CMake 3.8+ +- OpenSSL +- Node.js + Yarn 1.22 (for ISL/addons) + +## Git Workflow + +- Branch from `main` +- Developed internally at Meta, exported to GitHub +- CLA required for contributions (https://code.facebook.com/cla) diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 0000000000000..47dc3e3d863cf --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/addons/components/KeyboardShortcuts.tsx b/addons/components/KeyboardShortcuts.tsx index ddb14cce93491..9d7bcb13b2b5a 100644 --- a/addons/components/KeyboardShortcuts.tsx +++ b/addons/components/KeyboardShortcuts.tsx @@ -43,6 +43,7 @@ export enum KeyCode { R = 82, S = 83, T = 84, + Comma = 188, Period = 190, QuestionMark = 191, SingleQuote = 222, diff --git a/addons/isl-server/src/analytics/eventNames.ts b/addons/isl-server/src/analytics/eventNames.ts index 87a2bb2553714..3981327433543 100644 --- a/addons/isl-server/src/analytics/eventNames.ts +++ b/addons/isl-server/src/analytics/eventNames.ts @@ -106,6 +106,7 @@ export type TrackEventName = | 'PrSubmitOperation' | 'PullOperation' | 'PullRevOperation' + | 'PullStackOperation' | 'PurgeOperation' | 'RebaseWarningTimeout' | 'RebaseKeepOperation' diff --git a/addons/isl-server/src/github/githubCodeReviewProvider.ts b/addons/isl-server/src/github/githubCodeReviewProvider.ts index 0796650cbcc5b..3ff358bd36f23 100644 --- a/addons/isl-server/src/github/githubCodeReviewProvider.ts +++ b/addons/isl-server/src/github/githubCodeReviewProvider.ts @@ -41,6 +41,7 @@ import { YourPullRequestsQuery, YourPullRequestsWithoutMergeQueueQuery, } from './generated/graphql'; +import {parseStackInfo, type StackEntry} from './parseStackInfo'; import queryGraphQL from './queryGraphQL'; export type GitHubDiffSummary = { @@ -60,6 +61,8 @@ export type GitHubDiffSummary = { head: Hash; /** Name of the branch on GitHub, which should match the local bookmark */ branchName?: string; + /** Stack info parsed from PR body Sapling footer. Top-to-bottom order (first = top of stack). */ + stackInfo?: StackEntry[]; }; const DEFAULT_GH_FETCH_TIMEOUT = 60_000; // 1 minute @@ -111,11 +114,14 @@ export class GitHubCodeReviewProvider implements CodeReviewProvider { private fetchYourPullRequestsGraphQL( includeMergeQueue: boolean, ): Promise { + // Calculate date 30 days ago for the updated filter + const thirtyDaysAgo = new Date(); + thirtyDaysAgo.setDate(thirtyDaysAgo.getDate() - 30); + const dateFilter = thirtyDaysAgo.toISOString().split('T')[0]; + const variables = { - // TODO: somehow base this query on the list of DiffIds - // This is not very easy with github's graphql API, which doesn't allow more than 5 "OR"s in a search query. - // But if we used one-query-per-diff we would reach rate limiting too quickly. - searchQuery: `repo:${this.codeReviewSystem.owner}/${this.codeReviewSystem.repo} is:pr author:@me`, + // Fetch all open PRs in the repo updated in the last 30 days + searchQuery: `repo:${this.codeReviewSystem.owner}/${this.codeReviewSystem.repo} is:pr is:open updated:>=${dateFilter}`, numToFetch: 50, }; if (includeMergeQueue) { @@ -151,6 +157,9 @@ export class GitHubCodeReviewProvider implements CodeReviewProvider { this.logger.warn(`PR #${id} is missing base or head ref, skipping.`); continue; } + // Parse stack info from the PR body (Sapling footer format) + const stackInfo = parseStackInfo(summary.body) ?? undefined; + map.set(id, { type: 'github', title: summary.title, @@ -174,6 +183,7 @@ export class GitHubCodeReviewProvider implements CodeReviewProvider { base: summary.baseRef.target.oid, head: summary.headRef.target.oid, branchName: summary.headRef.name, + stackInfo, }); } } diff --git a/addons/isl-server/src/github/parseStackInfo.ts b/addons/isl-server/src/github/parseStackInfo.ts new file mode 100644 index 0000000000000..f326cae91dde3 --- /dev/null +++ b/addons/isl-server/src/github/parseStackInfo.ts @@ -0,0 +1,115 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * Represents a single entry in a PR stack. + */ +export type StackEntry = { + /** True if this is the current PR (marked with arrow in footer) */ + isCurrent: boolean; + /** The PR number */ + prNumber: number; +}; + +/** + * The Sapling footer marker that indicates the start of stack info. + */ +const SAPLING_FOOTER_MARKER = '[//]: # (BEGIN SAPLING FOOTER)'; + +/** + * Legacy marker for backward compatibility. + */ +const LEGACY_STACK_MARKER = 'Stack created with [Sapling]'; + +/** + * Regex to match stack entries in the PR body. + * Matches lines like: + * * #123 + * * #123 (2 commits) + * * __->__ #42 + * * __->__ #42 (3 commits) + */ +const STACK_ENTRY_REGEX = /^\* (__->__ )?#(\d+).*$/; + +/** + * Parse stack info from PR body. Matches the Sapling footer format: + * + * Stack ordering (top-to-bottom as it appears in the PR body): + * - First entry = top of stack (newest commits) + * - Last entry = closest to trunk (oldest commits) + * + * Example footer: + * ``` + * --- + * [//]: # (BEGIN SAPLING FOOTER) + * Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](...). + * * #125 + * * __->__ #124 ← current PR (marked with arrow) + * * #123 + * * #122 ← closest to trunk (bottom of stack) + * ``` + * + * @param body The PR body text + * @returns Array of stack entries in same order (top-to-bottom), or null if no stack info found + */ +export function parseStackInfo(body: string): StackEntry[] | null { + if (!body) { + return null; + } + + const lines = body.split(/\r?\n/); + let inStackList = false; + const stackEntries: StackEntry[] = []; + + for (const line of lines) { + if (lineHasStackListMarker(line)) { + inStackList = true; + continue; + } + + if (inStackList) { + const match = STACK_ENTRY_REGEX.exec(line); + if (match) { + const [, arrow, number] = match; + stackEntries.push({ + isCurrent: Boolean(arrow), + prNumber: parseInt(number, 10), + }); + } else if (stackEntries.length > 0) { + // We've reached the end of the list (non-matching line after entries) + break; + } + } + } + + return stackEntries.length > 0 ? stackEntries : null; +} + +/** + * Check if a line indicates the start of the stack list. + */ +function lineHasStackListMarker(line: string): boolean { + return line === SAPLING_FOOTER_MARKER || line.startsWith(LEGACY_STACK_MARKER); +} + +/** + * Get the index of the current PR in the stack. + * @param stackEntries The parsed stack entries + * @returns The index of the current PR, or -1 if not found + */ +export function getCurrentPrIndex(stackEntries: StackEntry[]): number { + return stackEntries.findIndex(entry => entry.isCurrent); +} + +/** + * Get all PR numbers in the stack. + * @param stackEntries The parsed stack entries + * @returns Array of PR numbers in stack order (top-to-bottom) + */ +export function getStackPrNumbers(stackEntries: StackEntry[]): number[] { + return stackEntries.map(entry => entry.prNumber); +} diff --git a/addons/isl/src/App.tsx b/addons/isl/src/App.tsx index ca536a2aebf9c..797495ea25386 100644 --- a/addons/isl/src/App.tsx +++ b/addons/isl/src/App.tsx @@ -22,6 +22,7 @@ import {availableCwds, CwdSelections} from './CwdSelector'; import {Drawers} from './Drawers'; import {EmptyState} from './EmptyState'; import {useCommand} from './ISLShortcuts'; +import {PRDashboard} from './PRDashboard'; import {Internal} from './Internal'; import {TopBar} from './TopBar'; import {TopLevelAlerts} from './TopLevelAlert'; @@ -95,9 +96,22 @@ function ISLDrawers() { right: {...state.right, collapsed: !state.right.collapsed}, })); }); + useCommand('ToggleLeftSidebar', () => { + setDrawerState(state => ({ + ...state, + left: {...state.left, collapsed: !state.left.collapsed}, + })); + }); return ( + + PR Stacks + + } + left={} rightLabel={ <> diff --git a/addons/isl/src/ISLShortcuts.tsx b/addons/isl/src/ISLShortcuts.tsx index f685a1ebbf12f..3645030d4d542 100644 --- a/addons/isl/src/ISLShortcuts.tsx +++ b/addons/isl/src/ISLShortcuts.tsx @@ -21,6 +21,7 @@ const CMD = isMac ? Modifier.CMD : Modifier.CTRL; export const [ISLCommandContext, useCommand, dispatchCommand, allCommands] = makeCommandDispatcher({ OpenShortcutHelp: [Modifier.SHIFT, KeyCode.QuestionMark], ToggleSidebar: [CMD, KeyCode.Period], + ToggleLeftSidebar: [CMD, KeyCode.Comma], OpenUncommittedChangesComparisonView: [CMD, KeyCode.SingleQuote], OpenHeadChangesComparisonView: [[CMD, Modifier.SHIFT], KeyCode.SingleQuote], Escape: [Modifier.NONE, KeyCode.Escape], @@ -58,6 +59,7 @@ export const ISLShortcutLabels: Partial> = { Escape: t('Dismiss Tooltips and Popups'), OpenShortcutHelp: t('Open Shortcut Help'), ToggleSidebar: t('Toggle Commit Info Sidebar'), + ToggleLeftSidebar: t('Toggle PR Stacks Sidebar'), OpenUncommittedChangesComparisonView: t('Open Uncommitted Changes Comparison View'), OpenHeadChangesComparisonView: t('Open Head Changes Comparison View'), SelectAllCommits: t('Select All Commits'), diff --git a/addons/isl/src/PRDashboard.css b/addons/isl/src/PRDashboard.css new file mode 100644 index 0000000000000..f6fd480aac7cd --- /dev/null +++ b/addons/isl/src/PRDashboard.css @@ -0,0 +1,145 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +.pr-dashboard { + display: flex; + flex-direction: column; + height: 100%; + overflow: hidden; +} + +.pr-dashboard-header { + display: flex; + align-items: center; + justify-content: space-between; + padding: var(--pad); + border-bottom: 1px solid var(--subtle-hover-darken); + flex-shrink: 0; +} + +.pr-dashboard-title { + font-weight: bold; + font-size: 14px; +} + +.pr-dashboard-content { + flex: 1; + overflow-y: auto; + padding: var(--pad); +} + +.pr-dashboard-empty { + display: flex; + flex-direction: column; + align-items: center; + justify-content: center; + gap: var(--pad); + padding: calc(var(--pad) * 2); + color: var(--foreground-sub); + text-align: center; +} + +.pr-dashboard-empty .codicon { + font-size: 24px; + opacity: 0.5; +} + +/* Stack Card */ +.stack-card { + border: 1px solid var(--subtle-hover-darken); + border-radius: 4px; + margin-bottom: var(--pad); + background: var(--background); +} + +.stack-card-header { + display: flex; + align-items: center; + padding: var(--halfpad) var(--pad); + gap: var(--halfpad); + background: var(--subtle-hover-darken); + border-radius: 4px 4px 0 0; +} + +.stack-card-expand-button { + padding: 2px; + min-width: auto; +} + +.stack-card-title { + flex: 1; + font-weight: 500; + cursor: pointer; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.stack-card-pull-button { + gap: 4px; +} + +.stack-card-prs { + padding: var(--halfpad); +} + +/* PR Row */ +.pr-row { + display: flex; + align-items: center; + padding: var(--halfpad); + gap: var(--halfpad); + border-radius: 2px; +} + +.pr-row:hover { + background: var(--subtle-hover-darken); +} + +.pr-row-status { + width: 16px; + text-align: center; + flex-shrink: 0; +} + +.pr-row-number { + color: var(--foreground-sub); + text-decoration: none; + flex-shrink: 0; +} + +.pr-row-number:hover { + text-decoration: underline; +} + +.pr-row-title { + flex: 1; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +/* PR State Colors */ +.pr-state-open { + color: var(--signal-good); +} + +.pr-state-merged { + color: var(--scm-modified-foreground); +} + +.pr-state-closed { + color: var(--signal-bad); +} + +.pr-state-draft { + color: var(--foreground-sub); +} + +.pr-state-queued { + color: var(--signal-warning); +} diff --git a/addons/isl/src/PRDashboard.tsx b/addons/isl/src/PRDashboard.tsx new file mode 100644 index 0000000000000..c3de5ed0a75d5 --- /dev/null +++ b/addons/isl/src/PRDashboard.tsx @@ -0,0 +1,154 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import type {PRStack} from './codeReview/PRStacksAtom'; +import type {DiffSummary} from './types'; + +import {Button} from 'isl-components/Button'; +import {Icon} from 'isl-components/Icon'; +import {Tooltip} from 'isl-components/Tooltip'; +import {useAtomValue} from 'jotai'; +import {useState} from 'react'; +import serverAPI from './ClientToServerAPI'; +import {prStacksAtom} from './codeReview/PRStacksAtom'; +import {T} from './i18n'; +import {useRunOperation} from './operationsState'; +import {PullStackOperation} from './operations/PullStackOperation'; + +import './PRDashboard.css'; + +export function PRDashboard() { + const stacks = useAtomValue(prStacksAtom); + + const handleRefresh = () => { + serverAPI.postMessage({type: 'fetchDiffSummaries'}); + }; + + return ( +
+
+ + PR Stacks + + + + +
+
+ {stacks.length === 0 ? ( +
+ + + No pull requests found + +
+ ) : ( + stacks.map(stack => ) + )} +
+
+ ); +} + +function StackCard({stack}: {stack: PRStack}) { + const [isExpanded, setIsExpanded] = useState(true); + const runOperation = useRunOperation(); + + const handlePullStack = () => { + runOperation(new PullStackOperation(stack.topPrNumber, /* goto */ true)); + }; + + const toggleExpanded = () => { + setIsExpanded(prev => !prev); + }; + + const headerTitle = stack.isStack + ? `Stack (${stack.prs.length} PRs)` + : `PR #${stack.topPrNumber}`; + + return ( +
+
+ + + {headerTitle} + + + + +
+ {isExpanded && ( +
+ {stack.prs.map(pr => ( + + ))} +
+ )} +
+ ); +} + +function PRRow({pr}: {pr: DiffSummary}) { + const stateIcon = getPRStateIcon(pr.state); + const stateClass = getPRStateClass(pr.state); + + return ( +
+ {stateIcon} + e.stopPropagation()}> + #{pr.number} + + + {pr.title} + +
+ ); +} + +function getPRStateIcon(state: DiffSummary['state']): string { + switch (state) { + case 'MERGED': + return '✓'; + case 'CLOSED': + return '✕'; + case 'DRAFT': + return '○'; + case 'MERGE_QUEUED': + return '◐'; + case 'OPEN': + default: + return '●'; + } +} + +function getPRStateClass(state: DiffSummary['state']): string { + switch (state) { + case 'MERGED': + return 'pr-state-merged'; + case 'CLOSED': + return 'pr-state-closed'; + case 'DRAFT': + return 'pr-state-draft'; + case 'MERGE_QUEUED': + return 'pr-state-queued'; + case 'OPEN': + default: + return 'pr-state-open'; + } +} diff --git a/addons/isl/src/codeReview/PRStacksAtom.ts b/addons/isl/src/codeReview/PRStacksAtom.ts new file mode 100644 index 0000000000000..6b7dbd509977d --- /dev/null +++ b/addons/isl/src/codeReview/PRStacksAtom.ts @@ -0,0 +1,131 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import type {DiffSummary} from '../types'; + +import {atom} from 'jotai'; +import {allDiffSummaries} from './CodeReviewInfo'; + +/** + * Represents a stack of PRs grouped together. + */ +export type PRStack = { + /** Unique identifier for this stack (usually the top PR number) */ + id: string; + /** The top PR number (first in the stack list) */ + topPrNumber: number; + /** All PR summaries in the stack, ordered top-to-bottom (first = top of stack) */ + prs: DiffSummary[]; + /** Whether this stack has multiple PRs or is just a single PR */ + isStack: boolean; +}; + +/** + * Groups PRs into stacks based on their stackInfo. + * + * PRs with matching stackInfo are grouped together. PRs without stackInfo + * are treated as single-PR stacks. + * + * Stack ordering: + * - First entry in stackInfo = top of stack (newest commits) + * - Last entry in stackInfo = closest to trunk (oldest commits) + */ +export const prStacksAtom = atom(get => { + const allDiffs = get(allDiffSummaries); + + if (allDiffs.error || allDiffs.value == null) { + return []; + } + + const diffsMap = allDiffs.value; + const stacks: PRStack[] = []; + const processedPrNumbers = new Set(); + + // Process each PR and group by stack + for (const [diffId, summary] of diffsMap.entries()) { + if (processedPrNumbers.has(diffId)) { + continue; + } + + const stackInfo = getStackInfo(summary); + + if (stackInfo && stackInfo.length > 1) { + // This PR is part of a multi-PR stack + // Build the stack from the stackInfo + const stackPrs: DiffSummary[] = []; + let topPrNumber: number | null = null; + + for (const entry of stackInfo) { + const prDiffId = String(entry.prNumber); + const prSummary = diffsMap.get(prDiffId); + + if (prSummary) { + stackPrs.push(prSummary); + processedPrNumbers.add(prDiffId); + + if (topPrNumber === null) { + topPrNumber = entry.prNumber; + } + } + } + + if (stackPrs.length > 0 && topPrNumber !== null) { + stacks.push({ + id: `stack-${topPrNumber}`, + topPrNumber, + prs: stackPrs, + isStack: stackPrs.length > 1, + }); + } + } else { + // Single PR (no stack info or single-entry stack) + const prNumber = parseInt(diffId, 10); + processedPrNumbers.add(diffId); + + stacks.push({ + id: `single-${diffId}`, + topPrNumber: prNumber, + prs: [summary], + isStack: false, + }); + } + } + + // Sort stacks by top PR number (descending - newest first) + stacks.sort((a, b) => b.topPrNumber - a.topPrNumber); + + return stacks; +}); + +/** + * Extract stackInfo from a DiffSummary. + * Only GitHub PRs have stackInfo. + */ +function getStackInfo( + summary: DiffSummary, +): Array<{isCurrent: boolean; prNumber: number}> | undefined { + if (summary.type === 'github' && 'stackInfo' in summary) { + return summary.stackInfo; + } + return undefined; +} + +/** + * Atom to get just the count of stacks. + */ +export const prStacksCountAtom = atom(get => { + const stacks = get(prStacksAtom); + return stacks.length; +}); + +/** + * Atom to get count of multi-PR stacks (excludes single PRs). + */ +export const multiPrStacksCountAtom = atom(get => { + const stacks = get(prStacksAtom); + return stacks.filter(s => s.isStack).length; +}); diff --git a/addons/isl/src/operations/PullStackOperation.ts b/addons/isl/src/operations/PullStackOperation.ts new file mode 100644 index 0000000000000..6c1d92e8a7f7c --- /dev/null +++ b/addons/isl/src/operations/PullStackOperation.ts @@ -0,0 +1,39 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {Operation} from './Operation'; + +/** + * Operation to pull an entire PR stack using `sl pr get`. + * + * This operation discovers and imports a full stack of PRs based on the + * stack information in the PR body. + */ +export class PullStackOperation extends Operation { + static opName = 'PullStack'; + + constructor( + private prNumber: number, + private goto: boolean = true, + ) { + super('PullStackOperation'); + } + + getArgs() { + const args = ['pr', 'get', String(this.prNumber)]; + if (this.goto) { + args.push('--goto'); + } + return args; + } + + getDescriptionForDisplay() { + return { + description: `Pull stack for PR #${this.prNumber}`, + }; + } +} diff --git a/eden/contrib/reviewstack/src/PullRequestChangeCount.tsx b/eden/contrib/reviewstack/src/PullRequestChangeCount.tsx index d7e93e8acf375..4510355573af8 100644 --- a/eden/contrib/reviewstack/src/PullRequestChangeCount.tsx +++ b/eden/contrib/reviewstack/src/PullRequestChangeCount.tsx @@ -5,23 +5,46 @@ * LICENSE file in the root directory of this source tree. */ -import {gitHubPullRequest} from './recoil'; -import {CounterLabel} from '@primer/react'; +import {gitHubPullRequest, pullRequestSLOC} from './recoil'; +import {CounterLabel, Tooltip} from '@primer/react'; import {useRecoilValue} from 'recoil'; export default function PullRequestChangeCount(): React.ReactElement | null { const pullRequest = useRecoilValue(gitHubPullRequest); + const slocInfo = useRecoilValue(pullRequestSLOC); if (pullRequest == null) { return null; } const {additions, deletions} = pullRequest; + const {significantLines, generatedFileCount} = slocInfo; + + const tooltipText = + generatedFileCount > 0 + ? `${significantLines} significant lines (excludes ${generatedFileCount} generated file${generatedFileCount === 1 ? '' : 's'})` + : `${significantLines} significant lines`; return ( <> - +{additions} - -{deletions} + +{additions} + + -{deletions} + + {significantLines > 0 && ( + + + {significantLines} sig + + + )} ); } diff --git a/eden/contrib/reviewstack/src/SplitDiffFileHeader.tsx b/eden/contrib/reviewstack/src/SplitDiffFileHeader.tsx index 7d1befb2ce2c1..8b16d12ed1ebd 100644 --- a/eden/contrib/reviewstack/src/SplitDiffFileHeader.tsx +++ b/eden/contrib/reviewstack/src/SplitDiffFileHeader.tsx @@ -6,16 +6,18 @@ */ import {ChevronDownIcon, ChevronRightIcon} from '@primer/octicons-react'; -import {Box, Text, Tooltip} from '@primer/react'; +import {Box, Label, Text, Tooltip} from '@primer/react'; export function FileHeader({ path, open, onChangeOpen, + isGenerated, }: { path: string; open?: boolean; onChangeOpen?: (open: boolean) => void; + isGenerated?: boolean; }) { // Even though the enclosing will have border-radius set, we // have to define it again here or things don't look right. @@ -79,6 +81,19 @@ export function FileHeader({ )} {filePathParts} + {isGenerated && ( + + + + )} ); } diff --git a/eden/contrib/reviewstack/src/SplitDiffView.tsx b/eden/contrib/reviewstack/src/SplitDiffView.tsx index d3df75f5b9351..c6c11f5ec84da 100644 --- a/eden/contrib/reviewstack/src/SplitDiffView.tsx +++ b/eden/contrib/reviewstack/src/SplitDiffView.tsx @@ -23,7 +23,9 @@ import SplitDiffRow from './SplitDiffRow'; import {diffAndTokenize, lineRange} from './diffServiceClient'; import {DiffSide} from './generated/graphql'; import {grammars, languages} from './generated/textmate/TextMateGrammarManifest'; +import {GeneratedStatus} from './github/types'; import { + fileGeneratedStatus, gitHubPullRequestLineToPositionForFile, gitHubPullRequestSelectedVersionIndex, gitHubPullRequestVersions, @@ -38,7 +40,7 @@ import {UnfoldIcon} from '@primer/octicons-react'; import {Box, Spinner, Text} from '@primer/react'; import {diffChars} from 'diff'; import React, {useCallback, useEffect, useState} from 'react'; -import {useRecoilValue, useRecoilValueLoadable, waitForAll} from 'recoil'; +import {constSelector, useRecoilValue, useRecoilValueLoadable, waitForAll} from 'recoil'; import organizeLinesIntoGroups from 'shared/SplitDiffView/organizeLinesIntoGroups'; import { applyTokenizationToLine, @@ -81,7 +83,6 @@ export default function SplitDiffView({ after, isPullRequest, }: Props): React.ReactElement { - const [open, setOpen] = useState(true); const scopeName = getFilepathClassifier().findScopeNameForPath(path); const colorMode = useRecoilValue(primerColorMode); const loadable = useRecoilValueLoadable( @@ -104,14 +105,32 @@ export default function SplitDiffView({ isPullRequest ? gitHubPullRequestVersions : nullAtom, isPullRequest ? gitHubPullRequestSelectedVersionIndex : nullAtom, isPullRequest ? gitHubPullRequestLineToPositionForFile(path) : nullAtom, + // Check if file is generated to default collapse state for generated files + isPullRequest ? fileGeneratedStatus(path) : constSelector(GeneratedStatus.Manual), ]), ); - const [{patch, tokenization}, allThreads, newCommentInputCallbacks, commitIDs] = - loadable.getValue(); + const [ + {patch, tokenization}, + allThreads, + newCommentInputCallbacks, + commitIDs, + _versions, + _selectedVersionIndex, + _lineToPosition, + generatedStatus, + ] = loadable.getValue(); + const isGenerated = generatedStatus === GeneratedStatus.Generated; + // Default to collapsed for generated files + const [open, setOpen] = useState(!isGenerated); return ( - setOpen(open)} /> + setOpen(open)} + isGenerated={isGenerated} + /> {open && ( (1500); + +/** + * localStorage key prefix for caching .gitattributes patterns per repo. + */ +const GITATTRIBUTES_CACHE_KEY_PREFIX = 'reviewstack:gitattributes:'; + +/** + * Detect if a file is generated by scanning its content for markers. + * Checks the first 1024 bytes for @generated or @partially-generated tags. + */ +export function detectGeneratedFromContent(content: string | null): GeneratedStatus { + if (content == null) { + return GeneratedStatus.Manual; + } + + // Only scan first 1024 bytes + const header = content.slice(0, 1024); + + if (header.includes('@generated')) { + return GeneratedStatus.Generated; + } + + if (header.includes('@partially-generated')) { + return GeneratedStatus.PartiallyGenerated; + } + + return GeneratedStatus.Manual; +} + +/** + * Detect if a file is generated based on its blob content. + * Results are cached by blob OID. + */ +export function detectGeneratedFromBlob(blob: Blob): GeneratedStatus { + // Check cache first + const cached = generatedStatusCache.get(blob.oid); + if (cached !== undefined) { + return cached; + } + + // Binary files are not considered generated (can't scan them) + if (blob.isBinary) { + generatedStatusCache.set(blob.oid, GeneratedStatus.Manual); + return GeneratedStatus.Manual; + } + + const status = detectGeneratedFromContent(blob.text); + generatedStatusCache.set(blob.oid, status); + return status; +} + +/** + * Check if a file path matches any of the default generated patterns. + */ +export function matchesDefaultGeneratedPattern(filePath: string): boolean { + return DEFAULT_GENERATED_PATTERNS.some(pattern => pattern.test(filePath)); +} + +/** + * Convert a gitattributes glob pattern to a RegExp. + * Supports: + * - ** matches any path segment(s) + * - * matches any characters except / + * - ? matches any single character + * - Leading / anchors to repo root + */ +export function gitAttributePatternToRegex(pattern: string): RegExp { + let regexStr = ''; + let i = 0; + + // Handle leading / (anchor to root) + const anchorToRoot = pattern.startsWith('/'); + if (anchorToRoot) { + regexStr += '^'; + i = 1; + } + + while (i < pattern.length) { + const char = pattern[i]; + + if (char === '*') { + if (pattern[i + 1] === '*') { + // ** matches any path including / + if (pattern[i + 2] === '/') { + // **/ at start or after / means zero or more directories + regexStr += '(?:.*/)?'; + i += 3; + } else { + // ** matches everything + regexStr += '.*'; + i += 2; + } + } else { + // * matches any characters except / + regexStr += '[^/]*'; + i += 1; + } + } else if (char === '?') { + // ? matches any single character except / + regexStr += '[^/]'; + i += 1; + } else if (char === '.') { + regexStr += '\\.'; + i += 1; + } else if (char === '/') { + regexStr += '/'; + i += 1; + } else { + // Escape other regex special characters + regexStr += char.replace(/[\\^$+{}[\]|()]/g, '\\$&'); + i += 1; + } + } + + // If pattern doesn't start with /, it can match at any depth + if (!anchorToRoot) { + regexStr = '(?:^|/)' + regexStr; + } + + // Anchor to end + regexStr += '$'; + + return new RegExp(regexStr); +} + +/** + * Parse .gitattributes file content and extract linguist-generated patterns. + * + * Format: each line is "pattern attribute=value" or "pattern attribute" + * We look for: linguist-generated, linguist-generated=true, -linguist-generated + */ +export function parseGitAttributes(content: string): GitAttributePattern[] { + const patterns: GitAttributePattern[] = []; + + for (const line of content.split('\n')) { + const trimmed = line.trim(); + + // Skip comments and empty lines + if (trimmed === '' || trimmed.startsWith('#')) { + continue; + } + + // Split into pattern and attributes + // Pattern can contain spaces if quoted, but we'll handle simple cases + const match = trimmed.match(/^(\S+)\s+(.+)$/); + if (!match) { + continue; + } + + const [, pattern, attributesStr] = match; + + // Check for linguist-generated attribute + // Formats: linguist-generated, linguist-generated=true, -linguist-generated + if (/\blinguist-generated\b/.test(attributesStr)) { + const isNegated = attributesStr.includes('-linguist-generated'); + const isFalse = /linguist-generated=false/.test(attributesStr); + + patterns.push({ + pattern, + isGenerated: !isNegated && !isFalse, + }); + } + } + + return patterns; +} + +/** + * Check if a file path matches any gitattributes pattern. + * Returns the matched pattern's isGenerated value, or null if no match. + */ +export function matchGitAttributePatterns( + filePath: string, + patterns: GitAttributePattern[], +): boolean | null { + // Process patterns in order - last match wins (gitattributes semantics) + let result: boolean | null = null; + + for (const {pattern, isGenerated} of patterns) { + const regex = gitAttributePatternToRegex(pattern); + if (regex.test(filePath)) { + result = isGenerated; + } + } + + return result; +} + +/** + * Get the generated status for a file, checking (in order): + * 1. .gitattributes patterns + * 2. File content (@generated tag) + * 3. Default patterns + */ +export function getGeneratedStatus( + filePath: string, + gitAttributePatterns: GitAttributePattern[], + blob: Blob | null, +): GeneratedStatus { + // 1. Check gitattributes patterns first + const gitAttrMatch = matchGitAttributePatterns(filePath, gitAttributePatterns); + if (gitAttrMatch === true) { + return GeneratedStatus.Generated; + } + if (gitAttrMatch === false) { + // Explicitly marked as not generated + return GeneratedStatus.Manual; + } + + // 2. Check blob content for @generated tag + if (blob != null) { + const blobStatus = detectGeneratedFromBlob(blob); + if (blobStatus !== GeneratedStatus.Manual) { + return blobStatus; + } + } + + // 3. Check default patterns + if (matchesDefaultGeneratedPattern(filePath)) { + return GeneratedStatus.Generated; + } + + return GeneratedStatus.Manual; +} + +/** + * Cache .gitattributes patterns in localStorage. + */ +export function cacheGitAttributePatterns( + owner: string, + repo: string, + commitOid: GitObjectID, + patterns: GitAttributePattern[], +): void { + const key = `${GITATTRIBUTES_CACHE_KEY_PREFIX}${owner}/${repo}:${commitOid}`; + try { + localStorage.setItem(key, JSON.stringify(patterns)); + } catch { + // localStorage might be full or unavailable + } +} + +/** + * Retrieve cached .gitattributes patterns from localStorage. + */ +export function getCachedGitAttributePatterns( + owner: string, + repo: string, + commitOid: GitObjectID, +): GitAttributePattern[] | null { + const key = `${GITATTRIBUTES_CACHE_KEY_PREFIX}${owner}/${repo}:${commitOid}`; + try { + const cached = localStorage.getItem(key); + if (cached != null) { + return JSON.parse(cached) as GitAttributePattern[]; + } + } catch { + // localStorage might be unavailable or corrupted + } + return null; +} + +/** + * Clear old .gitattributes cache entries to prevent localStorage bloat. + * Keeps only the most recent entries per repo. + */ +export function cleanupGitAttributesCache(maxEntriesPerRepo = 5): void { + try { + const entries: Array<{key: string; repo: string; time: number}> = []; + + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i); + if (key?.startsWith(GITATTRIBUTES_CACHE_KEY_PREFIX)) { + const repoMatch = key.match(/^reviewstack:gitattributes:([^:]+):/); + if (repoMatch) { + entries.push({ + key, + repo: repoMatch[1], + time: i, // Use index as rough time ordering + }); + } + } + } + + // Group by repo and keep only most recent + const byRepo = new Map(); + for (const entry of entries) { + const repoEntries = byRepo.get(entry.repo) ?? []; + repoEntries.push(entry); + byRepo.set(entry.repo, repoEntries); + } + + for (const repoEntries of byRepo.values()) { + if (repoEntries.length > maxEntriesPerRepo) { + // Sort by time (most recent last) and remove old entries + repoEntries.sort((a, b) => a.time - b.time); + for (let i = 0; i < repoEntries.length - maxEntriesPerRepo; i++) { + localStorage.removeItem(repoEntries[i].key); + } + } + } + } catch { + // Ignore cleanup failures + } +} diff --git a/eden/contrib/reviewstack/src/github/types.ts b/eden/contrib/reviewstack/src/github/types.ts index d091931291618..b8457d1b05ffe 100644 --- a/eden/contrib/reviewstack/src/github/types.ts +++ b/eden/contrib/reviewstack/src/github/types.ts @@ -173,3 +173,42 @@ export type PaginationParams = last: number; before?: PageInfo['startCursor']; }; + +/** + * Status indicating whether a file is generated (machine-created) or manual (human-written). + * Used for calculating "significant lines of code" by excluding generated files. + */ +export enum GeneratedStatus { + /** File is manually written by humans */ + Manual = 0, + /** File contains both generated and manual sections (has @partially-generated tag) */ + PartiallyGenerated = 1, + /** File is fully generated (has @generated tag or matches linguist-generated pattern) */ + Generated = 2, +} + +/** + * A pattern from .gitattributes that marks files as generated. + * See: https://github.com/github/linguist/blob/master/docs/overrides.md + */ +export interface GitAttributePattern { + /** Glob pattern (e.g., "*.generated.ts", "libs/graphql/**") */ + pattern: string; + /** Whether files matching this pattern are generated */ + isGenerated: boolean; +} + +/** + * Significant Lines of Code information for a pull request. + * Excludes generated files from the count to show meaningful diff statistics. + */ +export interface SLOCInfo { + /** Lines changed in non-generated files (additions + deletions) */ + significantLines: number; + /** Total lines changed across all files */ + totalLines: number; + /** Number of files detected as generated */ + generatedFileCount: number; + /** Paths of files detected as generated */ + generatedFiles: string[]; +} diff --git a/eden/contrib/reviewstack/src/recoil.ts b/eden/contrib/reviewstack/src/recoil.ts index 96fb80e85e449..2bf89341964e1 100644 --- a/eden/contrib/reviewstack/src/recoil.ts +++ b/eden/contrib/reviewstack/src/recoil.ts @@ -29,8 +29,10 @@ import type { Commit, DateTime, ForcePushEvent, + GitAttributePattern, GitObjectID, ID, + SLOCInfo, Version, VersionCommit, } from './github/types'; @@ -39,6 +41,12 @@ import type {RecoilValueReadOnly} from 'recoil'; import {lineToPosition} from './diffServiceClient'; import {DiffSide, PullRequestReviewState, UserHomePageQuery} from './generated/graphql'; +import { + cacheGitAttributePatterns, + getCachedGitAttributePatterns, + getGeneratedStatus, + parseGitAttributes, +} from './generatedFiles'; import CachingGitHubClient, {openDatabase} from './github/CachingGitHubClient'; import GraphQLGitHubClient from './github/GraphQLGitHubClient'; import {diffCommits, diffCommitWithParent} from './github/diff'; @@ -50,6 +58,7 @@ import { gitHubUsername, } from './github/gitHubCredentials'; import queryGraphQL from './github/queryGraphQL'; +import {GeneratedStatus} from './github/types'; import {stackedPullRequest, stackedPullRequestFragments} from './stackState'; import {getPathForChange, getTreeEntriesForChange, groupBy, groupByDiffSide} from './utils'; import {atom, atomFamily, constSelector, selector, selectorFamily, waitForAll} from 'recoil'; @@ -1307,3 +1316,198 @@ function createClient( const client = new GraphQLGitHubClient(hostname, organization, repository, token); return new CachingGitHubClient(db, client, organization, repository); } + +// ============================================================================= +// SLOC (Significant Lines of Code) selectors +// ============================================================================= + +/** + * Fetches and parses the .gitattributes file from the repo root. + * Uses localStorage cache to avoid re-fetching for the same commit. + */ +export const gitAttributesPatterns = selector({ + key: 'gitAttributesPatterns', + get: async ({get}) => { + const orgAndRepo = get(gitHubOrgAndRepo); + const commits = get(gitHubPullRequestCommits); + const client = get(gitHubClient); + + if (orgAndRepo == null || commits.length === 0 || client == null) { + return []; + } + + const {org, repo} = orgAndRepo; + // Use the latest commit's OID + const latestCommit = commits[commits.length - 1]; + const commitOid = latestCommit.oid; + + // Check localStorage cache first + const cached = getCachedGitAttributePatterns(org, repo, commitOid); + if (cached != null) { + return cached; + } + + // Fetch .gitattributes from the repo root + try { + // Get the tree for the head commit to find .gitattributes + const commit = await client.getCommit(commitOid); + if (commit == null) { + return []; + } + + // Find .gitattributes in the root tree + const gitattributesEntry = commit.tree.entries.find( + entry => entry.name === '.gitattributes' && entry.type === 'blob', + ); + + if (gitattributesEntry == null) { + // No .gitattributes file - cache empty result + cacheGitAttributePatterns(org, repo, commitOid, []); + return []; + } + + // Fetch the blob content + const blob = await client.getBlob(gitattributesEntry.oid); + if (blob == null || blob.isBinary || blob.text == null) { + cacheGitAttributePatterns(org, repo, commitOid, []); + return []; + } + + // Parse the .gitattributes content + const patterns = parseGitAttributes(blob.text); + cacheGitAttributePatterns(org, repo, commitOid, patterns); + return patterns; + } catch { + // On error, return empty patterns + return []; + } + }, +}); + +/** + * Get the generated status for a specific file in the current PR diff. + * Checks .gitattributes patterns, file content, and default patterns. + */ +export const fileGeneratedStatus = selectorFamily({ + key: 'fileGeneratedStatus', + get: + (filePath: string) => + ({get}) => { + const gitAttrPatterns = get(gitAttributesPatterns); + const diff = get(gitHubPullRequestVersionDiff); + + if (diff == null) { + return GeneratedStatus.Manual; + } + + // Find the file in the diff to get its blob OID + const fileChange = diff.diff.find(change => getPathForChange(change) === filePath); + + // Get the blob for content scanning + let blob: Blob | null = null; + if (fileChange != null) { + const entries = getTreeEntriesForChange(fileChange); + // Prefer the "after" version, fall back to "before" for deletions + const blobOid = entries.after?.oid ?? entries.before?.oid; + if (blobOid != null) { + blob = get(gitHubBlob(blobOid)); + } + } + + return getGeneratedStatus(filePath, gitAttrPatterns, blob); + }, +}); + +/** + * Calculate SLOC (Significant Lines of Code) for the current PR. + * Excludes generated files from the count. + * + * Note: This selector intentionally calls getGeneratedStatus() directly instead + * of using the fileGeneratedStatus selectorFamily to avoid creating deep + * dependency chains that can cause performance issues with many files. + */ +export const pullRequestSLOC = selector({ + key: 'pullRequestSLOC', + get: ({get}) => { + const pullRequest = get(gitHubPullRequest); + const diff = get(gitHubPullRequestVersionDiff); + // Load gitAttributesPatterns once, not per-file via fileGeneratedStatus + const gitAttrPatterns = get(gitAttributesPatterns); + + if (pullRequest == null || diff == null) { + return { + significantLines: 0, + totalLines: 0, + generatedFileCount: 0, + generatedFiles: [], + }; + } + + // Use the PR-level additions/deletions as the total + // Note: This counts all lines, not just non-blank lines + const totalLines = pullRequest.additions + pullRequest.deletions; + + let generatedLines = 0; + const generatedFiles: string[] = []; + + for (const change of diff.diff) { + const filePath = getPathForChange(change); + const entries = getTreeEntriesForChange(change); + + // Get blob for generated status detection (prefer after, fall back to before) + const blobOid = entries.after?.oid ?? entries.before?.oid; + const blob = blobOid != null ? get(gitHubBlob(blobOid)) : null; + + // Check if file is generated using the function directly to avoid + // deep dependency chains from fileGeneratedStatus selectorFamily + const generatedStatus = getGeneratedStatus(filePath, gitAttrPatterns, blob); + if (generatedStatus === GeneratedStatus.Generated) { + generatedFiles.push(filePath); + + // Count diff lines (additions + deletions) for this generated file + // This must be compatible with how totalLines is calculated (PR additions + deletions) + let fileAdditions = 0; + let fileDeletions = 0; + + if (change.type === 'add' && entries.after != null) { + // New file: all non-blank lines are additions + const afterBlob = get(gitHubBlob(entries.after.oid)); + if (afterBlob?.text != null) { + fileAdditions = afterBlob.text.split('\n').filter(line => line.trim() !== '').length; + } + } else if (change.type === 'remove' && entries.before != null) { + // Deleted file: all non-blank lines are deletions + const beforeBlob = get(gitHubBlob(entries.before.oid)); + if (beforeBlob?.text != null) { + fileDeletions = beforeBlob.text.split('\n').filter(line => line.trim() !== '').length; + } + } else if (change.type === 'modify') { + // Modified file: approximate by comparing before/after line counts + const beforeBlob = entries.before != null ? get(gitHubBlob(entries.before.oid)) : null; + const afterBlob = entries.after != null ? get(gitHubBlob(entries.after.oid)) : null; + + const beforeLines = + beforeBlob?.text?.split('\n').filter(line => line.trim() !== '').length ?? 0; + const afterLines = + afterBlob?.text?.split('\n').filter(line => line.trim() !== '').length ?? 0; + + fileAdditions = Math.max(0, afterLines - beforeLines); + fileDeletions = Math.max(0, beforeLines - afterLines); + } + + generatedLines += fileAdditions + fileDeletions; + } + } + + // Significant lines = total - generated + // This is an approximation since we're using PR-level totals + const significantLines = Math.max(0, totalLines - generatedLines); + + return { + significantLines, + totalLines, + generatedFileCount: generatedFiles.length, + generatedFiles, + }; + }, +}); diff --git a/eden/scm/sapling/ext/github/__init__.py b/eden/scm/sapling/ext/github/__init__.py index 4fde178b404c4..9839bfec5126b 100644 --- a/eden/scm/sapling/ext/github/__init__.py +++ b/eden/scm/sapling/ext/github/__init__.py @@ -16,6 +16,7 @@ from . import ( follow, + get_stack, github_repo_util, import_pull_request, link, @@ -75,8 +76,8 @@ def pull_request_command(ui, repo, *args, **opts): subcmd = pull_request_command.subcommand( categories=[ ( - "Create or update pull requests, using `pull` to import a PR, if necessary", - ["submit", "pull"], + "Create or update pull requests, using `pull` or `get` to import PRs", + ["submit", "pull", "get"], ), ( "Manually manage associations with pull requests", @@ -108,7 +109,11 @@ def submit_cmd(ui, repo, *args, **opts): Pull request(s) will be created against ``default``. If ``default`` is a fork, they will be created against default's - upstream repository. + upstream repository by default. To create PRs on your fork instead, + set:: + + [github] + submit-to-upstream = false Returns 0 on success. """ @@ -139,6 +144,42 @@ def pull_cmd(ui, repo, *args, **opts): return import_pull_request.get_pr(ui, repo, *args, **opts) +@subcmd( + "get", + [ + ( + "g", + "goto", + False, + _("goto the target pull request after importing the stack"), + ), + ( + "", + "downstack", + False, + _("only fetch PRs from target towards trunk (skip upstack)"), + ), + ], + _("PULL_REQUEST"), +) +def get_cmd(ui, repo, *args, **opts): + """import an entire PR stack into your working copy + + The PULL_REQUEST can be specified as either a URL: + `https://github.com/facebook/sapling/pull/321` + or just the PR number within the GitHub repository identified by + `sl config paths.default`. + + Unlike `sl pr pull` which imports only a single PR, this command + discovers and imports the entire stack of PRs based on the stack + information in the PR body. + + Use --downstack to fetch only PRs from the target towards trunk, + skipping any upstack (descendant) PRs. + """ + return get_stack.get(ui, repo, *args, **opts) + + @subcmd( "link", [("r", "rev", "", _("revision to link"), _("REV"))], diff --git a/eden/scm/sapling/ext/github/get_stack.py b/eden/scm/sapling/ext/github/get_stack.py new file mode 100644 index 0000000000000..7f60df557a29a --- /dev/null +++ b/eden/scm/sapling/ext/github/get_stack.py @@ -0,0 +1,146 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +"""Implementation of `sl pr get` command to pull entire PR stacks from GitHub. + +This command is similar to `sl pr pull` but fetches an entire stack of PRs +instead of just a single PR. + +Usage: + sl pr get 123 # Pull stack for PR #123 + sl pr get https://github.com/.../pull/123 + sl pr get 123 --goto # Pull and checkout target + sl pr get 123 --downstack # Only fetch trunk→target (skip upstack) +""" + +import asyncio + +from sapling import error +from sapling.hg import updatetotally +from sapling.i18n import _ +from sapling.node import bin + +from .consts.stackheader import STACK_HEADER_PREFIX as GHSTACK_HEADER_PREFIX +from .pull_request_arg import parse_pull_request_arg +from .pullrequest import PullRequestId +from .pullrequeststore import PullRequestStore +from .stack_discovery import discover_stack_from_pr, get_head_nodes + + +def get(ui, repo, *args, **opts): + """Pull an entire PR stack from GitHub. + + The PULL_REQUEST can be specified as either a URL: + `https://github.com/facebook/sapling/pull/321` + or just the PR number within the GitHub repository identified by + `sl config paths.default`. + + This command differs from `sl pr pull` in that it fetches the entire + stack of PRs, not just a single PR. + """ + if len(args) == 0: + raise error.Abort( + _("PR URL or number must be specified. See '@prog@ pr get -h'.") + ) + + pr_arg = args[0] + pr_id = parse_pull_request_arg(pr_arg, repo=repo) + if pr_id is None: + raise error.Abort( + _("Could not parse pull request arg: '%s'. Specify PR by URL or number.") + % pr_arg + ) + + is_goto = opts.get("goto") + downstack_only = opts.get("downstack") + + return asyncio.run( + _get_stack(ui, repo, pr_id=pr_id, is_goto=is_goto, downstack_only=downstack_only) + ) + + +async def _get_stack( + ui, + repo, + pr_id: PullRequestId, + is_goto: bool, + downstack_only: bool, +) -> int: + """Internal implementation of stack fetch. + + Returns 0 on success. + """ + ui.status(_("discovering stack for PR #%d...\n") % pr_id.number) + + # Check for ghstack PRs which need different handling + from .gh_submit import get_pull_request_details + + initial_result = await get_pull_request_details(pr_id) + if initial_result.is_err(): + raise error.Abort( + _("could not get pull request details: %s") % initial_result.unwrap_err() + ) + + initial_details = initial_result.unwrap() + if _looks_like_ghstack_pull_request(initial_details.body): + raise error.Abort( + _( + "This pull request appears to be part of a ghstack stack.\n" + "Try running the following instead:\n" + " sl ghstack checkout %s" + ) + % pr_id.as_url() + ) + + # Discover the full stack + try: + stack_result = await discover_stack_from_pr(pr_id, downstack_only=downstack_only) + except RuntimeError as e: + raise error.Abort(_("stack discovery failed: %s") % str(e)) + + num_prs = len(stack_result.entries) + if num_prs == 1: + ui.status(_("found single PR (no stack)\n")) + else: + mode = "downstack" if downstack_only else "full stack" + ui.status(_("found %d PRs in %s\n") % (num_prs, mode)) + + # Get all head nodes for pulling + head_nodes = get_head_nodes(stack_result) + + # Pull all commits + ui.status(_("pulling %d commit(s)...\n") % len(head_nodes)) + repo.pull(headnodes=tuple(head_nodes)) + + # Link all commits to their respective PRs + store = PullRequestStore(repo) + target_node = None + + for entry in stack_result.entries: + node = bin(entry.head_oid) + store.map_commit_to_pull_request(node, entry.pr_id) + + if entry.is_target: + target_node = node + + ui.status(_("imported #%d as %s\n") % (entry.number, entry.head_oid[:12])) + + ui.status(_("successfully imported %d PR(s)\n") % num_prs) + + # Goto target if requested + if is_goto and target_node is not None: + target_entry = stack_result.entries[stack_result.target_index] + updatetotally(ui, repo, target_node, None) + ui.status(_("now at #%d\n") % target_entry.number) + + return 0 + + +def _looks_like_ghstack_pull_request(body: str) -> bool: + """Check if a PR body indicates it was created by ghstack.""" + for line in body.splitlines(): + if line.startswith(GHSTACK_HEADER_PREFIX): + return True + return False diff --git a/eden/scm/sapling/ext/github/gh_submit.py b/eden/scm/sapling/ext/github/gh_submit.py index f3a62a085d365..bf2e7109c91a4 100644 --- a/eden/scm/sapling/ext/github/gh_submit.py +++ b/eden/scm/sapling/ext/github/gh_submit.py @@ -46,16 +46,21 @@ class Repository: # then we only traverse one link in the chain, so this could still be None. upstream: Optional["Repository"] = None - def get_base_branch(self) -> str: - """If this is a fork, returns the default_branch of the upstream repo.""" - if self.upstream: + def get_base_branch(self, use_upstream: bool = True) -> str: + """If this is a fork and use_upstream is True, returns the default_branch + of the upstream repo; otherwise returns this repo's default_branch.""" + if use_upstream and self.upstream: return self.upstream.default_branch else: return self.default_branch - def get_upstream_owner_and_name(self) -> Tuple[str, str]: - """owner and name to use when creating a pull request""" - if self.upstream: + def get_upstream_owner_and_name(self, use_upstream: bool = True) -> Tuple[str, str]: + """owner and name to use when creating a pull request. + + If use_upstream is True and this is a fork, returns the upstream's + owner and name. Otherwise returns this repo's owner and name. + """ + if use_upstream and self.upstream: return (self.upstream.owner, self.upstream.name) else: return (self.owner, self.name) diff --git a/eden/scm/sapling/ext/github/stack_discovery.py b/eden/scm/sapling/ext/github/stack_discovery.py new file mode 100644 index 0000000000000..18d1a28e6f2ea --- /dev/null +++ b/eden/scm/sapling/ext/github/stack_discovery.py @@ -0,0 +1,175 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +"""Stack discovery logic for `sl pr get` command. + +This module discovers the full stack of PRs from a given PR's body by parsing +the Sapling footer format. + +Stack ordering: +- The stack list in PR bodies is **top-to-bottom** +- First entry = top of stack (newest commits) +- Last entry = closest to trunk (oldest commits) +""" + +import asyncio +from dataclasses import dataclass +from typing import List, Optional + +from .gh_submit import get_pull_request_details, PullRequestDetails +from .pull_request_body import parse_stack_information +from .pullrequest import PullRequestId + + +@dataclass +class StackEntry: + """Represents a single PR in the stack.""" + + pr_id: PullRequestId + """The pull request identifier.""" + + number: int + """The PR number.""" + + head_oid: str + """The head commit OID for this PR.""" + + is_target: bool + """True if this is the target PR that the user requested.""" + + details: PullRequestDetails + """The full PR details from GitHub.""" + + +@dataclass +class StackDiscoveryResult: + """Result of stack discovery.""" + + entries: List[StackEntry] + """Ordered list of stack entries (top-to-bottom, first = top of stack).""" + + target_index: int + """Index of the target PR in the entries list.""" + + +async def discover_stack_from_pr( + pr_id: PullRequestId, + downstack_only: bool = False, +) -> StackDiscoveryResult: + """Discover the full stack from a PR's body. + + Args: + pr_id: The target PR to start discovery from. + downstack_only: If True, only fetch PRs from target towards trunk + (target index to end of list). + + Returns: + StackDiscoveryResult with ordered entries and target index. + + The stack is ordered top-to-bottom as it appears in the PR body: + - entries[0] is the top of the stack (newest commits) + - entries[-1] is closest to trunk (oldest commits) + """ + # First, get the target PR details + result = await get_pull_request_details(pr_id) + if result.is_err(): + raise RuntimeError(f"could not get pull request details: {result.unwrap_err()}") + + target_details = result.unwrap() + body = target_details.body + + # Parse stack information from the PR body + stack_entries = parse_stack_information(body) + + if not stack_entries: + # No stack info in body - treat as single-PR stack + entry = StackEntry( + pr_id=pr_id, + number=pr_id.number, + head_oid=target_details.head_oid, + is_target=True, + details=target_details, + ) + return StackDiscoveryResult(entries=[entry], target_index=0) + + # Find the target PR in the stack list + # Stack entries are List[Tuple[is_current: bool, number: int]] + target_index = None + for i, (is_current, number) in enumerate(stack_entries): + if is_current or number == pr_id.number: + target_index = i + break + + if target_index is None: + # Target not found in stack - maybe stack info is stale + # Fall back to single-PR behavior + entry = StackEntry( + pr_id=pr_id, + number=pr_id.number, + head_oid=target_details.head_oid, + is_target=True, + details=target_details, + ) + return StackDiscoveryResult(entries=[entry], target_index=0) + + # Determine which PRs to fetch based on downstack_only flag + if downstack_only: + # Fetch from target index to end (towards trunk) + entries_to_fetch = stack_entries[target_index:] + adjusted_target_index = 0 + else: + # Fetch all entries + entries_to_fetch = stack_entries + adjusted_target_index = target_index + + # Fetch all PR details in parallel + async def fetch_entry(index: int, is_current: bool, number: int) -> StackEntry: + entry_pr_id = PullRequestId( + hostname=pr_id.hostname, + owner=pr_id.owner, + name=pr_id.name, + number=number, + ) + + # Use cached details for target PR + if number == pr_id.number: + details = target_details + else: + result = await get_pull_request_details(entry_pr_id) + if result.is_err(): + raise RuntimeError( + f"could not get details for PR #{number}: {result.unwrap_err()}" + ) + details = result.unwrap() + + is_target = index == adjusted_target_index if downstack_only else is_current or number == pr_id.number + + return StackEntry( + pr_id=entry_pr_id, + number=number, + head_oid=details.head_oid, + is_target=is_target, + details=details, + ) + + entries = await asyncio.gather( + *[ + fetch_entry(i, is_current, number) + for i, (is_current, number) in enumerate(entries_to_fetch) + ] + ) + + return StackDiscoveryResult(entries=list(entries), target_index=adjusted_target_index) + + +def get_head_nodes(result: StackDiscoveryResult) -> List[bytes]: + """Get the head commit nodes for all entries in the stack. + + Returns: + List of binary node IDs for all stack entry head commits. + """ + from sapling.node import bin + + return [bin(entry.head_oid) for entry in result.entries] diff --git a/eden/scm/sapling/ext/github/submit.py b/eden/scm/sapling/ext/github/submit.py index f7f617e443045..9a1e4476dfa30 100644 --- a/eden/scm/sapling/ext/github/submit.py +++ b/eden/scm/sapling/ext/github/submit.py @@ -33,8 +33,11 @@ def submit(ui, repo, *args, **opts) -> int: """Create or update GitHub pull requests.""" github_repo = check_github_repo(repo) is_draft = opts.get("draft") + submit_to_upstream = ui.configbool("github", "submit-to-upstream", True) return asyncio.run( - update_commits_in_stack(ui, repo, github_repo, is_draft=is_draft) + update_commits_in_stack( + ui, repo, github_repo, is_draft=is_draft, submit_to_upstream=submit_to_upstream + ) ) @@ -148,7 +151,7 @@ async def get_partitions(ui, repo, store, filter) -> List[List[CommitData]]: async def update_commits_in_stack( - ui, repo, github_repo: GitHubRepo, is_draft: bool + ui, repo, github_repo: GitHubRepo, is_draft: bool, submit_to_upstream: bool = True ) -> int: parents = repo.dirstate.parents() if parents[0] == nullid: @@ -167,11 +170,11 @@ async def update_commits_in_stack( use_placeholder_strategy = ui.configbool("github", "placeholder-strategy") if use_placeholder_strategy: params = await create_placeholder_strategy_params( - ui, partitions, github_repo, origin + ui, partitions, github_repo, origin, submit_to_upstream ) else: params = await create_serial_strategy_params( - ui, partitions, github_repo, origin + ui, partitions, github_repo, origin, submit_to_upstream ) max_pull_requests_to_create = ui.configint("github", "max-prs-to-create", "5") @@ -224,6 +227,7 @@ def get_gitdir() -> str: store, ui, is_draft, + submit_to_upstream, ) else: assert isinstance(params, SerialStrategyParams) @@ -234,6 +238,7 @@ def get_gitdir() -> str: store, ui, is_draft, + submit_to_upstream, ) # Now that each pull request has a named branch pushed to GitHub, we can @@ -249,7 +254,8 @@ def get_gitdir() -> str: repository = await get_repository_for_origin(origin, github_repo.hostname) rewrite_and_archive_requests = [ rewrite_pull_request_body( - partitions, index, workflow, pr_numbers_and_num_commits, repository, ui + partitions, index, workflow, pr_numbers_and_num_commits, repository, ui, + submit_to_upstream ) for index in range(len(partitions)) ] + [ @@ -272,12 +278,13 @@ async def rewrite_pull_request_body( pr_numbers_and_num_commits: List[Tuple[int, int]], repository: Repository, ui, + submit_to_upstream: bool = True, ) -> None: # If available, use the head branch of the previous partition as the base # of this branch. Recall that partitions is ordered from the top of the # stack to the bottom. partition = partitions[index] - base = repository.get_base_branch() + base = repository.get_base_branch(submit_to_upstream) if workflow == SubmitWorkflow.SINGLE and index < len(partitions) - 1: base = none_throws(partitions[index + 1][0].head_branch_name) @@ -364,6 +371,7 @@ async def create_serial_strategy_params( partitions: List[List[CommitData]], github_repo: GitHubRepo, origin: str, + submit_to_upstream: bool = True, ) -> SerialStrategyParams: # git push --force any heads that need updating, creating new branch names, # if necessary. @@ -394,7 +402,9 @@ async def create_serial_strategy_params( repository = await get_repository_for_origin( origin, github_repo.hostname ) - upstream_owner, upstream_name = repository.get_upstream_owner_and_name() + upstream_owner, upstream_name = repository.get_upstream_owner_and_name( + submit_to_upstream + ) result = await gh_submit.guess_next_pull_request_number( github_repo.hostname, upstream_owner, upstream_name ) @@ -429,14 +439,15 @@ async def create_pull_requests_serially( store: PullRequestStore, ui, is_draft: bool, + submit_to_upstream: bool = True, ) -> None: """Creates a new pull request for each entry in the `commits` list. Each CommitData in `commits` will be updated such that its `.pr` field is set appropriately. """ - head_ref_prefix = f"{repository.owner}:" if repository.is_fork else "" - owner, name = repository.get_upstream_owner_and_name() + head_ref_prefix = f"{repository.owner}:" if repository.is_fork and submit_to_upstream else "" + owner, name = repository.get_upstream_owner_and_name(submit_to_upstream) hostname = repository.hostname # Create the pull requests in order serially to give us the best chance of @@ -444,7 +455,7 @@ async def create_pull_requests_serially( commits_to_update = [] parent = None for commit, branch_name in commits: - base = repository.get_base_branch() + base = repository.get_base_branch(submit_to_upstream) if workflow == SubmitWorkflow.SINGLE and parent: base = none_throws(parent.head_branch_name) @@ -505,6 +516,7 @@ async def create_placeholder_strategy_params( partitions: List[List[CommitData]], github_repo: GitHubRepo, origin: str, + submit_to_upstream: bool = True, ) -> PlaceholderStrategyParams: refs_to_update: List[str] = [] commits_that_need_pull_requests: List[CommitNeedsPullRequest] = [] @@ -543,7 +555,7 @@ async def create_placeholder_strategy_params( if commits_that_need_pull_requests: repository = await get_repository_for_origin(origin, github_repo.hostname) issue_numbers = await _create_placeholder_issues( - repository, len(commits_that_need_pull_requests) + repository, len(commits_that_need_pull_requests), submit_to_upstream ) for commit_needs_pr, number in zip( commits_that_need_pull_requests, issue_numbers @@ -574,15 +586,16 @@ async def create_pull_requests_from_placeholder_issues( store: PullRequestStore, ui, is_draft: bool, + submit_to_upstream: bool = True, ) -> None: """Creates a new pull request for each entry in the `commits` list. Each entry in `commits` is a (CommitData, branch_name, issue_number). Each CommitData will be updated such that its `.pr` field is set appropriately. """ - head_ref_prefix = f"{repository.owner}:" if repository.is_fork else "" - owner, name = repository.get_upstream_owner_and_name() - base_branch_for_repo = repository.get_base_branch() + head_ref_prefix = f"{repository.owner}:" if repository.is_fork and submit_to_upstream else "" + owner, name = repository.get_upstream_owner_and_name(submit_to_upstream) + base_branch_for_repo = repository.get_base_branch(submit_to_upstream) hostname = repository.hostname async def create_pull_request(params: PullRequestParams): @@ -643,9 +656,13 @@ async def create_pull_request(params: PullRequestParams): await asyncio.gather(*[create_pull_request(c) for c in commits]) -async def _create_placeholder_issues(repository: Repository, num: int) -> List[int]: +async def _create_placeholder_issues( + repository: Repository, num: int, submit_to_upstream: bool = True +) -> List[int]: """create the specified number of placeholder issues in parallel""" - upstream_owner, upstream_name = repository.get_upstream_owner_and_name() + upstream_owner, upstream_name = repository.get_upstream_owner_and_name( + submit_to_upstream + ) issue_number_results = await asyncio.gather( *[ gh_submit.create_pull_request_placeholder_issue( diff --git a/eden/scm/tests/test-ext-github-pr-get.t b/eden/scm/tests/test-ext-github-pr-get.t new file mode 100644 index 0000000000000..9a9a3ab574e00 --- /dev/null +++ b/eden/scm/tests/test-ext-github-pr-get.t @@ -0,0 +1,56 @@ +#require no-eden + +#inprocess-hg-incompatible + + $ eagerepo + $ enable github + +Test help output for `sl pr get` + + $ hg help pr get + hg pr get PULL_REQUEST + + import an entire PR stack into your working copy + + The PULL_REQUEST can be specified as either a URL: + 'https://github.com/facebook/sapling/pull/321' or just the PR number + within the GitHub repository identified by 'sl config paths.default'. + + Unlike 'sl pr pull' which imports only a single PR, this command discovers + and imports the entire stack of PRs based on the stack information in the + PR body. + + Use --downstack to fetch only PRs from the target towards trunk, skipping + any upstack (descendant) PRs. + + Options: + + -g --goto goto the target pull request after importing the stack + --downstack only fetch PRs from target towards trunk (skip upstack) + + (some details hidden, use --verbose to show complete help) + +Test error without argument + + $ hg init repo + $ cd repo + $ echo a > a1 + $ hg ci -Am addfile + adding a1 + + $ hg pr get + abort: PR URL or number must be specified. See 'hg pr get -h'. + [255] + +Test error for non-github repo + + $ hg pr get 123 + abort: This does not appear to be a GitHub repo + [255] + +Test pr get appears in pr subcommand list + + $ hg help pr | grep "get" + get import an entire PR stack into your working copy + + $ cd ..