From bbaec83e27ff7ce4e7822919bc32de295eb26091 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 18:48:39 +0000 Subject: [PATCH 01/10] feat(package): declarative package management with a lock file Projects can now declare external dependencies in an RBTPackage.json next to Source/. rbt resolves them transitively, materializes them under Packages/, and records what each pin actually resolved to in RBTPackage.lock.json. Restore runs from CppBuildProject.Parse(), before ParseRules(). That ordering is forced, not incidental: CompileRules.dll is loaded once with Assembly.LoadFile and cannot be unloaded, so a package's .module.cs has to be on disk before the rule glob runs - there is no second chance to add rules afterwards. The same constraint is why the manifest is JSON rather than C#: resolving transitive dependencies means repeatedly reading the manifest of a package that has not been downloaded yet. Packages contribute modules and extensions, never targets - what gets built stays the consuming project's decision. Their rules are globbed into the same rule assembly as the project's own, so a package module is depended on by name like any local one. Resolution is exact pins only, with no version solver: a git dependency must name a commit, tag or branch, conflicting pins for one package are a hard error pointing at the "overrides" escape hatch, and dependency cycles are reported with the whole chain. The lock stores the commit a tag resolved to, since upstream can move a tag, and is rewritten only when its content changes - rbt's incremental checks are timestamp based. Packages live in /Packages/ rather than under Intermedia/, which Clean() empties and CleanIfNeed() wipes whenever the rbt binaries are newer than the last build; putting them there would re-download every dependency after each rebuild and each rbt update. Sources in this change are git and path. PackageArgs adds --Offline, --ForceRestore and --UpdateLock; it lives in CppCompiler because CmdParser discovers argument groups by scanning loaded assemblies and .NET loads them lazily. RunMode gains Restore. CppTargetRule.GitLibraries, the unread predecessor of all this, is marked [Obsolete]: it hangs off a target rule, which only exists after the rule assembly is compiled, so it never could have worked. A project without an RBTPackage.json is entirely unaffected - no Packages/ directory, no lock file, no .gitignore edit. Tests: manifest validation and lock behaviour, the graph walk (transitive, diamond, cycle, conflict, override), and an end-to-end restore that clones a real git repository created in a temp dir. All offline, so the three CI hosts stay deterministic. Sample/PackageConsumer consumes Sample/GeometryPackage over a path dependency and is compiled and linked for real by the existing sample build suite. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019nEaEVhDjn2dQC19BE9iaM --- Doc/ARCH.md | 62 ++++- Doc/ARCH.zh-CN.md | 53 +++- Doc/HowToUse.md | 101 +++++++- Doc/HowToUse.zh-CN.md | 92 ++++++- README.md | 1 + .../Common/CppTargetRule.cs | 1 + .../Project/CppBuildProject.cs | 54 ++++- .../Project/PackageArgs.cs | 35 +++ .../CommandGroup/ICommonCommandGroup.cs | 9 +- .../CompileService/CppCompile.cs | 7 + .../Context/ServiceContext.Default.cs | 4 + .../Fetchers/GitPackageFetcher.cs | 147 ++++++++++++ .../Fetchers/IPackageFetcher.cs | 61 +++++ .../Fetchers/PathPackageFetcher.cs | 39 +++ .../PackageService/IPackageService.cs | 62 +++++ .../PackageService/PackageLock.cs | 102 ++++++++ .../PackageService/PackageManifest.cs | 226 ++++++++++++++++++ .../PackageService/PackageResolver.cs | 160 +++++++++++++ .../PackageService/PackageRestoreService.cs | 87 +++++++ .../PackageService/ProcessRunner.cs | 105 ++++++++ ReBuildTool/ReBuildTool.Test/TestCppBuild.cs | 5 +- .../ReBuildTool.Test/TestPackageManifest.cs | 176 ++++++++++++++ .../ReBuildTool.Test/TestPackageResolver.cs | 167 +++++++++++++ .../ReBuildTool.Test/TestPackageRestore.cs | 199 +++++++++++++++ ReBuildTool/ReBuildTool/Program.cs | 3 + .../GeometryPackage/GeometryModule.module.cs | 19 ++ .../Private/GeometryModule.cpp | 11 + .../GeometryPackage/Public/GeometryModule.h | 10 + Sample/GeometryPackage/RBTPackage.json | 3 + Sample/PackageConsumer/.gitignore | 6 + Sample/PackageConsumer/RBTPackage.json | 6 + Sample/PackageConsumer/RBTPackage.lock.json | 13 + .../Source/AppModule/AppModule.module.cs | 14 ++ .../Source/AppModule/Private/AppModule.cpp | 20 ++ .../Source/AppModule/Public/AppModule.h | 7 + .../Source/PackageConsumerTarget.target.cs | 9 + Sample/PackageConsumer/global.json | 7 + 37 files changed, 2067 insertions(+), 16 deletions(-) create mode 100644 ReBuildTool/ReBuildTool.CppCompiler/Project/PackageArgs.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/PathPackageFetcher.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/IPackageService.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/PackageLock.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs create mode 100644 ReBuildTool/ReBuildTool.Test/TestPackageManifest.cs create mode 100644 ReBuildTool/ReBuildTool.Test/TestPackageResolver.cs create mode 100644 ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs create mode 100644 Sample/GeometryPackage/GeometryModule.module.cs create mode 100644 Sample/GeometryPackage/Private/GeometryModule.cpp create mode 100644 Sample/GeometryPackage/Public/GeometryModule.h create mode 100644 Sample/GeometryPackage/RBTPackage.json create mode 100644 Sample/PackageConsumer/.gitignore create mode 100644 Sample/PackageConsumer/RBTPackage.json create mode 100644 Sample/PackageConsumer/RBTPackage.lock.json create mode 100644 Sample/PackageConsumer/Source/AppModule/AppModule.module.cs create mode 100644 Sample/PackageConsumer/Source/AppModule/Private/AppModule.cpp create mode 100644 Sample/PackageConsumer/Source/AppModule/Public/AppModule.h create mode 100644 Sample/PackageConsumer/Source/PackageConsumerTarget.target.cs create mode 100644 Sample/PackageConsumer/global.json diff --git a/Doc/ARCH.md b/Doc/ARCH.md index bd4d318..f6ce168 100644 --- a/Doc/ARCH.md +++ b/Doc/ARCH.md @@ -34,7 +34,7 @@ Arrows in the dependency graph point from a project to what it **depends on**. | **ReBuildTool.CppCompiler** | The core. Parses C++ build rules, resolves modules/targets, drives compile + link + archive through platform toolchains and SDKs. ~9k LOC. | | **ReBuildTool.CSharpCompiler** | Compiles the user's `*.target.cs` / `*.module.cs` rule files into a `CompileRules.dll` at runtime (Roslyn-based). | | **ReBuildTool.IDE** | Generators that emit Visual Studio (`.vcxproj`/`.sln`) and CMake projects. | -| **ReBuildTool.Ini** | The `IIniProject` implementation — an alternate, INI-driven project front end run alongside the C++ project. | +| ~~**ReBuildTool.Ini**~~ | Removed. `Program.cs` no longer creates an `IIniProject`; `ServiceContext.Config.cs::InitFromIni()` remains as a stub that returns `false`. | | **ReBuildTool.Common** | Shared utilities: `Shell` (process wrapper), `NiceIO` path helpers, misc. | | **ReBuildTool.Updater** | Standalone self-update executable, shipped next to `rbt`. | | **ReBuildTool.CppCompiler.Standalone** | Thin standalone host around the C++ compiler for isolated runs/testing. | @@ -121,6 +121,49 @@ These rule files are **not** compiled ahead of time. At runtime recompiled when a rule file (or `rbt` itself) is newer — with a bounded retry counter for load failures. +### 4.1 Package management and why it runs first + +`CppBuildProject.Parse()` calls `RestorePackages()` **before** `ParseRules()`. +That ordering is forced by the step above: `CompileRules.dll` is loaded exactly +once with `Assembly.LoadFile` and .NET cannot unload it, so there is no way to +compile rules, discover a dependency, and then add its rules to the same +assembly. Every package's `.module.cs` therefore has to be on disk before the +glob runs. + +That constraint is also why the manifest is JSON rather than C#: resolving +transitive dependencies means repeatedly reading the manifest of a package that +has not been downloaded yet, which a plain file read does trivially and a +compile-and-load cycle cannot. + +``` +Parse() + ├─ RestorePackages() IPackageService (ReBuildTool.Service/PackageService) + │ ├─ read /RBTPackage.json (absent → returns immediately, zero cost) + │ ├─ PackageResolver: depth-first walk + │ │ fetch → read the package's own manifest → recurse + │ │ exact pins only; conflicting pins and cycles are hard errors + │ ├─ IPackageFetcher per source Git (clone/fetch/reset) | Path (used in place) + │ └─ write RBTPackage.lock.json only when changed + └─ ParseRules() globs Source/ + each restored package root +``` + +Key types, all in `ReBuildTool.Service/PackageService/`: `PackageManifest`, +`PackageLockFile`, `PackageResolver`, `PackageRestoreService`, and +`Fetchers/IPackageFetcher`. The `--Offline` / `--ForceRestore` / `--UpdateLock` +flags live in `ReBuildTool.CppCompiler/Project/PackageArgs.cs` — deliberately in +that assembly rather than beside the service, because `CmdParser` discovers +argument groups by scanning `AppDomain.CurrentDomain.GetAssemblies()` and .NET +loads assemblies lazily. + +Packages contribute **modules and extensions, never targets**: a `*.target.cs` +inside a package is ignored, so what gets built stays the consuming project's +decision. Restored packages land in `/Packages/`, not under +`Intermedia/` — see §8. + +`CppTargetRule.GitLibraries` is the superseded predecessor of all this. It is +marked `[Obsolete]` and never read; it could not have worked, for the ordering +reason above. + --- ## 5. End-to-end build flow @@ -264,6 +307,22 @@ platform / configuration / architecture, e.g.: ///ObjectCache/ per-source .obj/.o mirror of Source/ ``` +Restored packages deliberately sit **outside** that tree: + +``` +/ + RBTPackage.json dependency manifest, hand written + RBTPackage.lock.json resolved commits, generated, commit it + Packages// materialized package (git clone, or absent + for a path dependency, which is used in place) +``` + +`Packages/` is not under `Intermedia/` because `Clean()` empties that directory, +and `CleanIfNeed()` triggers a clean on its own whenever the rbt binaries are +newer than the last build — every dependency would be re-downloaded after each +rebuild and each rbt update. Restore adds `/Packages/` to the project's +`.gitignore`. + The `ObjectCache` mirrors the source tree so incremental timestamp checks (`IsCompileUnitUpToDate`) can map each source file to its object deterministically. @@ -289,6 +348,7 @@ The `ObjectCache` mirrors the source tree so incremental timestamp checks | CLI dispatch | `ReBuildTool/Program.cs` | | Service wiring | `ReBuildTool.Service/Context/ServiceContext*.cs` | | Rule compile + load | `ReBuildTool.CppCompiler/Project/CppBuildProject.cs` | +| Package restore / resolution | `ReBuildTool.Service/PackageService/` (start at `PackageResolver.cs`) | | Compile scheduling / incremental | `ReBuildTool.CppCompiler/Common/CppBuilder.Process.Compile.cs` | | Adding a toolchain | `ReBuildTool.CppCompiler/ToolChain/IToolChain.cs` + an existing `ToolChain//` | | SDK discovery | `ReBuildTool.CppCompiler/SDK/` | diff --git a/Doc/ARCH.zh-CN.md b/Doc/ARCH.zh-CN.md index 9292ce4..3d42e36 100644 --- a/Doc/ARCH.zh-CN.md +++ b/Doc/ARCH.zh-CN.md @@ -28,7 +28,7 @@ ReBuildTool 是一个用 **.NET 8** 编写的 C++ 构建系统，设计思路借 | **ReBuildTool.CppCompiler** | 核心。解析 C++ 构建规则、解析 module/target，通过各平台工具链与 SDK 驱动 compile + link + archive，约 9k 行。 | | **ReBuildTool.CSharpCompiler** | 在运行时把用户的 `*.target.cs` / `*.module.cs` 规则文件编译为 `CompileRules.dll`（基于 Roslyn）。 | | **ReBuildTool.IDE** | 生成器，输出 Visual Studio（`.vcxproj`/`.sln`）与 CMake 工程。 | -| **ReBuildTool.Ini** | `IIniProject` 实现——一套基于 INI 的备选 project 前端，与 C++ project 并行运行。 | +| ~~**ReBuildTool.Ini**~~ | 已移除。`Program.cs` 不再创建 `IIniProject`；`ServiceContext.Config.cs::InitFromIni()` 仅作为返回 `false` 的桩保留。 | | **ReBuildTool.Common** | 共享工具：`Shell`（进程封装）、`NiceIO` 路径辅助等。 | | **ReBuildTool.Updater** | 独立的自更新可执行文件，与 `rbt` 一起分发。 | | **ReBuildTool.CppCompiler.Standalone** | 围绕 C++ 编译器的轻量独立宿主，用于隔离运行 / 测试。 | @@ -102,6 +102,42 @@ CppCompiler CSharpCompiler IDE Ini `NeedReBuildRuleAssembly()` 通过比较时间戳，仅在某个规则文件（或 `rbt` 本身）更新时才重新 编译规则 DLL——并对加载失败设有次数上限的重试。 +### 4.1 包管理，以及它为什么必须跑在最前面 + +`CppBuildProject.Parse()` 会在 `ParseRules()` **之前**调用 `RestorePackages()`。 +这个顺序是被上一节的机制强制的：`CompileRules.dll` 用 `Assembly.LoadFile` 只加载一次， +.NET 无法卸载它，因此不存在「先编译规则、再发现依赖、再把依赖的规则加进同一个程序集」 +这条路。所有包的 `.module.cs` 都必须在 glob 之前就位于磁盘上。 + +同样的约束决定了清单格式必须是 JSON 而不是 C#：解析传递依赖意味着反复读取「尚未下载的包」 +的清单，普通文件读取轻而易举，而「编译 + 加载」的循环做不到。 + +``` +Parse() + ├─ RestorePackages() IPackageService（ReBuildTool.Service/PackageService） + │ ├─ 读取 /RBTPackage.json （不存在则立即返回，零开销） + │ ├─ PackageResolver：深度优先遍历 + │ │ 拉取 → 读取该包自己的清单 → 递归 + │ │ 只接受精确 pin；pin 冲突与依赖成环均为硬错误 + │ ├─ 按来源分派 IPackageFetcher Git（clone/fetch/reset）| Path（原地使用） + │ └─ 写出 RBTPackage.lock.json 仅在内容变化时 + └─ ParseRules() glob Source/ 以及每个已 restore 的包根目录 +``` + +主要类型都在 `ReBuildTool.Service/PackageService/` 下：`PackageManifest`、 +`PackageLockFile`、`PackageResolver`、`PackageRestoreService`，以及 +`Fetchers/IPackageFetcher`。`--Offline` / `--ForceRestore` / `--UpdateLock` 三个参数 +定义在 `ReBuildTool.CppCompiler/Project/PackageArgs.cs` —— 特意放在该程序集而不是服务旁边， +因为 `CmdParser` 通过扫描 `AppDomain.CurrentDomain.GetAssemblies()` 发现参数组， +而 .NET 的程序集是惰性加载的。 + +包提供的是**模块和 extension，绝不是 target**：包里的 `*.target.cs` 会被忽略， +构建什么始终由消费方项目决定。restore 出来的包放在 `/Packages/`， +不在 `Intermedia/` 下 —— 见 §8。 + +`CppTargetRule.GitLibraries` 是这套机制被取代掉的前身。它已标记 `[Obsolete]` 且从未被读取； +基于上面的顺序原因，它本来也不可能工作。 + --- ## 5. 端到端构建流程 @@ -225,6 +261,20 @@ stdout/stderr 重定向到日志，并且由于并行编译共享一个非线程 `ObjectCache` 镜像源码树，使增量时间戳检查（`IsCompileUnitUpToDate`）能确定性地把每个源文件 映射到它的目标文件。 +restore 出来的包特意放在这棵树**之外**： + +``` +/ + RBTPackage.json 依赖清单，手写 + RBTPackage.lock.json 解析到的 commit，工具生成，应当提交 + Packages// 物化后的包（git clone；path 依赖原地使用， + 不会出现在这里） +``` + +`Packages/` 不放在 `Intermedia/` 下，是因为 `Clean()` 会清空该目录，而且只要 rbt 的二进制 +比上次构建新，`CleanIfNeed()` 就会自行触发一次 clean —— 那样每次 rebuild、每次 rbt 升级 +都要重新下载全部依赖。restore 会把 `/Packages/` 加进项目的 `.gitignore`。 + --- ## 9. 分发与更新 @@ -245,6 +295,7 @@ stdout/stderr 重定向到日志，并且由于并行编译共享一个非线程 | CLI 分派 | `ReBuildTool/Program.cs` | | 服务装配 | `ReBuildTool.Service/Context/ServiceContext*.cs` | | 规则编译 + 加载 | `ReBuildTool.CppCompiler/Project/CppBuildProject.cs` | +| 包 restore / 解析 | `ReBuildTool.Service/PackageService/`（从 `PackageResolver.cs` 读起） | | 编译调度 / 增量 | `ReBuildTool.CppCompiler/Common/CppBuilder.Process.Compile.cs` | | 新增工具链 | `ReBuildTool.CppCompiler/ToolChain/IToolChain.cs` + 已有的 `ToolChain//` | | SDK 探测 | `ReBuildTool.CppCompiler/SDK/` | diff --git a/Doc/HowToUse.md b/Doc/HowToUse.md index 461553f..b184f01 100644 --- a/Doc/HowToUse.md +++ b/Doc/HowToUse.md @@ -75,7 +75,7 @@ ReBuildTool --ProjectRoot --Mode --Target [options...] | Flag | Meaning | |---|---| | `--ProjectRoot ` | Project root folder. Defaults to the current working directory. | -| `--Mode ` | **Required.** One of `Init`, `Build`, `Clean`, `ReBuild`. | +| `--Mode ` | **Required.** One of `Init`, `Build`, `Clean`, `ReBuild`, `Restore`. | | `--Target ` | Target to build. Defaults to the `ProjectRoot` folder name. | | `--IDEProjectType ` | Which project to generate in `Init` mode: `VisualStudio`, `CMake`, `VSCode`, or `CompileCommands`. Defaults to Visual Studio on Windows, CMake elsewhere. | | `--BoosterSource ` | Internal; set by the Booster scripts so RBT can regenerate them. Don't set manually. | @@ -98,6 +98,11 @@ ReBuildTool --ProjectRoot --Mode --Target [options...] - **Build** — compiles the given target. - **Clean** — removes build outputs. - **ReBuild** — `Clean` followed by `Build`. +- **Restore** — fetches the packages declared in `RBTPackage.json` and writes + the lock file, then stops. Every other mode restores implicitly first, so this + is only needed to populate a checkout up front (warming a CI cache, or pulling + dependencies down while a machine still has network). See + [§5 Package management](#5-package-management). ### C++-specific flags @@ -155,7 +160,11 @@ public class MyGameTarget : CppTargetRule Key members of `CppTargetRule`: - `List UsedModules` — modules linked into this target (entry points). -- `List GitLibraries` — external git dependencies (`Name`, `Url`, `Branch`). +- ~~`List GitLibraries`~~ — **deprecated and never read.** Declare + dependencies in `RBTPackage.json` instead ([§5](#5-package-management)). It + could not work where it sits: the list hangs off a target rule, which only + exists once the rule assembly has been compiled, whereas a package's own + `.module.cs` has to be on disk *before* that compile. - `List Plugins` — pre/post-compile hooks. - `virtual void Setup(ICppBuildContext)` / `virtual void PostBuild()` — override for custom logic. @@ -219,7 +228,89 @@ builds and auto-generates `.internal.h/.cpp` import/export macro pairs. MyGameModule.cpp ``` -## 5. Programmatic / lifecycle API +## 5. Package management + +A project declares its external dependencies in an `RBTPackage.json` next to +`Source/`. Before every build RBT fetches whatever is missing, materializes it +under `Packages/`, and records exactly what it resolved to in +`RBTPackage.lock.json`. + +```jsonc +{ + "name": "MyGame", + "dependencies": { + // a git package, pinned to a tag + "GreeterLib": { "git": "https://github.com/x/greeter.git", "tag": "v1.2.0" }, + // pinned to an exact commit + "FooLib": { "git": "https://github.com/x/foo.git", "commit": "a1b2c3d4..." }, + // a directory on this machine, for local co-development + "LocalLib": { "path": "../LocalLib" } + } +} +``` + +Each dependency sets **exactly one** source (`git` or `path`), and a git source +must carry a `commit`, `tag` or `branch` — RBT resolves exact pins only and will +never pick a version for you. + +### What a package is + +A package is just a directory containing `.module.cs` rule files. Once restored, +its rules are globbed into the very same `CompileRules.dll` as the project's own, +so a package module is depended on by name like any local one: + +```csharp +Dependencies.Add("GeometryModule"); +``` + +Packages contribute **modules, not targets** — what to build stays the consuming +project's decision, so a `*.target.cs` inside a package is ignored. A package's +name and its modules' names are independent; see +[Sample/PackageConsumer](../Sample/PackageConsumer) and the package it consumes, +[Sample/GeometryPackage](../Sample/GeometryPackage). + +### Transitive dependencies + +A package declares its own dependencies in its own `RBTPackage.json`, and RBT +walks the graph: fetch a package, read the manifest it brought with it, fetch +what that names, and so on. A package reachable twice is fetched once. + +Because there is no version solver, two packages pinning the same dependency +differently is a **hard error**. Name the winner explicitly in the root manifest: + +```jsonc +{ "overrides": { "FooLib": { "git": "https://github.com/x/foo.git", "tag": "v2.0" } } } +``` + +Dependency cycles are rejected with the whole chain named. + +### The lock file + +`RBTPackage.lock.json` records the commit each pin actually resolved to — a tag +can be moved upstream, a commit cannot — so a later restore reproduces the same +tree and needs no network. **Commit it.** It is rewritten only when its content +actually changes, so it does not churn your working tree. + +### Flags + +| Flag | Meaning | +|---|---| +| `--Offline` | Never access the network. Fails if the lock is not already satisfied on disk. | +| `--ForceRestore` | Re-fetch every package even when the lock is already satisfied. | +| `--UpdateLock` | Re-resolve moving pins (tags and branches) and rewrite the lock, like `cargo update`. | + +### Where things land + +`Packages/` sits in the project root, **not** under `Intermedia/`: `Clean` wipes +`Intermedia/`, and RBT cleans on its own whenever its binaries are newer than the +last build, so dependencies would be re-downloaded after every rebuild and every +RBT update. Restore adds `/Packages/` to the project's `.gitignore`; a `path` +dependency is used where it lies and never copied. + +A project with no `RBTPackage.json` is completely unaffected — no `Packages/` +directory, no lock file, no `.gitignore` edit. + +## 6. Programmatic / lifecycle API Every project exposes the same lifecycle, also used internally by `Program.cs`'s mode dispatch and by the NUnit tests in @@ -233,7 +324,7 @@ project.Clean(); project.ReBuild(targetName); ``` -## 6. Self-update +## 7. Self-update `ReBuildTool.Updater` (invoked via `rbt-updater.sh` / `rbt-updater.bat`) pulls the latest `ReBuildTool` git repo into `$RBT_HOME` and rebuilds it from @@ -244,7 +335,7 @@ repository: ./BuildScript/rbt-updater.sh ``` -## 7. Quick reference +## 8. Quick reference ```bash # one-time setup in an empty project folder diff --git a/Doc/HowToUse.zh-CN.md b/Doc/HowToUse.zh-CN.md index 7524e3d..81341fc 100644 --- a/Doc/HowToUse.zh-CN.md +++ b/Doc/HowToUse.zh-CN.md @@ -73,7 +73,7 @@ ReBuildTool --ProjectRoot --Mode --Target [options...] | 参数 | 含义 | |---|---| | `--ProjectRoot ` | 项目根目录，默认为当前工作目录。 | -| `--Mode ` | **必填。** 取值为 `Init`、`Build`、`Clean`、`ReBuild` 之一。 | +| `--Mode ` | **必填。** 取值为 `Init`、`Build`、`Clean`、`ReBuild`、`Restore` 之一。 | | `--Target ` | 要构建的目标名称，默认为 `ProjectRoot` 文件夹名。 | | `--IDEProjectType ` | `Init` 模式下生成哪种工程：`VisualStudio`、`CMake`、`VSCode` 或 `CompileCommands`。默认 Windows 为 Visual Studio，其他平台为 CMake。 | | `--BoosterSource ` | 内部参数，由 Booster 脚本设置，用于 RBT 重新生成这些脚本。请勿手动设置。 | @@ -93,6 +93,10 @@ ReBuildTool --ProjectRoot --Mode --Target [options...] - **Build** —— 编译指定的目标。 - **Clean** —— 清理构建产物。 - **ReBuild** —— 先 `Clean` 再 `Build`。 +- **Restore** —— 拉取 `RBTPackage.json` 里声明的包并写出 lock 文件，然后停止。 + 其它模式都会先隐式执行一次 restore，所以这个模式只用于提前把依赖准备好 + （预热 CI 缓存，或者趁机器还有网络时先把依赖拉下来）。 + 见 [§5 包管理](#5-包管理)。 ### C++ 相关的专用参数 @@ -150,7 +154,10 @@ public class MyGameTarget : CppTargetRule `CppTargetRule` 的主要成员： - `List UsedModules` —— 链接进该 Target 的模块（入口模块）。 -- `List GitLibraries` —— 外部 git 依赖库（`Name`、`Url`、`Branch`）。 +- ~~`List GitLibraries`~~ —— **已废弃，且从未被读取。** 请改用 + `RBTPackage.json` 声明依赖（[§5](#5-包管理)）。它待在这个位置上就不可能工作： + 该列表挂在 target rule 上，而 target rule 只有在规则程序集编译完成后才存在， + 但包自带的 `.module.cs` 必须在那次编译**之前**就落到磁盘上。 - `List Plugins` —— 编译前/后钩子。 - `virtual void Setup(ICppBuildContext)` / `virtual void PostBuild()` —— 可重写以实现自定义逻辑。 @@ -212,7 +219,82 @@ Target 规则不同：它的 `UsedModules` / `Plugins` 在任何 target `Setup` MyGameModule.cpp ``` -## 5. 编程式 / 生命周期 API +## 5. 包管理 + +项目在 `Source/` 旁边的 `RBTPackage.json` 里声明外部依赖。每次构建前 RBT 会拉取 +缺失的包，把它们物化到 `Packages/` 下，并把实际解析到的结果记录进 +`RBTPackage.lock.json`。 + +```jsonc +{ + "name": "MyGame", + "dependencies": { + // git 包，固定到某个 tag + "GreeterLib": { "git": "https://github.com/x/greeter.git", "tag": "v1.2.0" }, + // 固定到精确 commit + "FooLib": { "git": "https://github.com/x/foo.git", "commit": "a1b2c3d4..." }, + // 本机上的目录，用于本地联调 + "LocalLib": { "path": "../LocalLib" } + } +} +``` + +每条依赖**有且只有一个**来源（`git` 或 `path`）；git 来源必须带上 `commit`、`tag` +或 `branch` —— RBT 只接受精确 pin，永远不会替你挑版本。 + +### 什么是一个包 + +包就是一个装着 `.module.cs` 规则文件的目录。restore 之后，它的规则会和项目自己的 +规则一起被 glob 进同一个 `CompileRules.dll`，因此包里的模块和本地模块一样按名字依赖： + +```csharp +Dependencies.Add("GeometryModule"); +``` + +包提供的是**模块，而不是 target** —— 构建什么始终由消费方项目决定，所以包里的 +`*.target.cs` 会被忽略。包名和它里面的模块名互相独立；参见 +[Sample/PackageConsumer](../Sample/PackageConsumer) 及它消费的 +[Sample/GeometryPackage](../Sample/GeometryPackage)。 + +### 传递依赖 + +包在自己的 `RBTPackage.json` 里声明自己的依赖，RBT 会沿着图往下走：拉一个包，读它 +带来的清单，再拉清单里点名的包，如此往复。被多条路径引用到的包只会拉取一次。 + +因为没有版本求解器，两个包对同一个依赖给出不同的 pin 属于**硬错误**。请在根清单里 +显式指定哪个胜出： + +```jsonc +{ "overrides": { "FooLib": { "git": "https://github.com/x/foo.git", "tag": "v2.0" } } } +``` + +依赖成环会被拒绝，并把整条链路打印出来。 + +### lock 文件 + +`RBTPackage.lock.json` 记录每个 pin 实际解析到的 commit —— tag 可能被上游移动， +commit 不会 —— 这样后续 restore 能复现同一棵树，且完全不需要网络。**请提交它。** +它只在内容真正变化时才重写，不会污染工作区。 + +### 相关参数 + +| 参数 | 作用 | +|---|---| +| `--Offline` | 绝不访问网络。若 lock 尚未在磁盘上被满足则直接失败。 | +| `--ForceRestore` | 即使 lock 已满足也重新拉取所有包。 | +| `--UpdateLock` | 重新解析会移动的 pin（tag / branch）并重写 lock，相当于 `cargo update`。 | + +### 东西放在哪 + +`Packages/` 位于项目根目录，**不在** `Intermedia/` 下：`Clean` 会清空 +`Intermedia/`，而且只要 RBT 的二进制比上次构建新，它就会自行触发一次 clean —— +放在那里意味着每次 rebuild、每次 RBT 升级都要重新下载全部依赖。restore 会把 +`/Packages/` 加进项目的 `.gitignore`；`path` 依赖在原地使用，不会被复制。 + +没有 `RBTPackage.json` 的项目完全不受影响 —— 不会有 `Packages/` 目录、不会有 lock +文件、也不会改动 `.gitignore`。 + +## 6. 编程式 / 生命周期 API 所有工程类型都暴露相同的生命周期方法，`Program.cs` 中的模式分发逻辑，以及 [ReBuildTool.Test](../ReBuildTool/ReBuildTool.Test) 中的 NUnit 测试内部都用到了它： @@ -225,7 +307,7 @@ project.Clean(); project.ReBuild(targetName); ``` -## 6. 自我更新 +## 7. 自我更新 `ReBuildTool.Updater`（通过 `rbt-updater.sh` / `rbt-updater.bat` 调用）会拉取 `$RBT_HOME` 下最新的 `ReBuildTool` git 仓库，并从源码重新构建 @@ -235,7 +317,7 @@ project.ReBuild(targetName); ./BuildScript/rbt-updater.sh ``` -## 7. 快速参考 +## 8. 快速参考 ```bash # 在空项目文件夹中做一次性初始化 diff --git a/README.md b/README.md index 5d66291..a14df62 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,7 @@ A C#-driven native build system in the spirit of Unreal Engine's UBT. Describe * - **C# rule files** — `.target.cs` / `.module.cs` describe your build graph; no separate DSL to learn - **Multi-toolchain** — MSVC (VS2017/2019/2022 auto-detect), Clang, GCC, Wasm - **Cross-platform** — Windows, Linux, macOS (x64 & arm64) +- **Package management** — declare git/path dependencies in `RBTPackage.json`; RBT resolves them transitively, pins them in a lock file and folds their modules into the build - **IDE integration** — generates Visual Studio `.sln` and CMake projects - **Self-updating** — `rbt-updater` rebuilds RBT from its own repo diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Common/CppTargetRule.cs b/ReBuildTool/ReBuildTool.CppCompiler/Common/CppTargetRule.cs index c6a40a9..a510114 100644 --- a/ReBuildTool/ReBuildTool.CppCompiler/Common/CppTargetRule.cs +++ b/ReBuildTool/ReBuildTool.CppCompiler/Common/CppTargetRule.cs @@ -12,6 +12,7 @@ public abstract class CppTargetRule : ITargetInterface, IPostBuildTarget public virtual Dictionary CustomInfo { get; } = new(); + [Obsolete("Declare dependencies in RBTPackage.json instead - this list is never read.")] public virtual List GitLibraries { get; } = new(); public ICppBuildContext BuildContext { get; private set; } diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs b/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs index e3a3ee0..ef50038 100644 --- a/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs +++ b/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs @@ -7,6 +7,7 @@ using ReBuildTool.Service.IDEService; using ReBuildTool.Service.IDEService.CMake; using ReBuildTool.Service.IDEService.VisualStudio; +using ReBuildTool.Service.PackageService; using ResetCore.Common; using ResetCore.Common.Parser.Ini; @@ -39,13 +40,24 @@ private void ParseRules() var moduleFiles = SourceFolder.Files($"*{ICppProject.ModuleDefineExtension}", true).ToList(); var extraFiles = SourceFolder.Files($"*{ICppProject.ExtensionDefineExtension}", true).ToList(); + // Only the project's own Source/ decides whether this is a fresh project needing a + // scaffold. A project that consumes packages but has not written its target yet still + // has to be initialized, so restored packages must not count here. if (targetFiles.Count == 0) { CreateDefaultProject(); ParseRules(); return; } - + + // Packages contribute modules, never targets: what to build is the consuming project's + // decision, and a package's target would otherwise silently join the build. + foreach (var package in RestoredPackages) + { + moduleFiles.AddRange(package.Root.Files($"*{ICppProject.ModuleDefineExtension}", true)); + extraFiles.AddRange(package.Root.Files($"*{ICppProject.ExtensionDefineExtension}", true)); + } + foreach (var targetFile in targetFiles) { TargetRulePaths.Add(targetFile.FileNameWithoutExtension, targetFile); @@ -54,7 +66,18 @@ private void ParseRules() foreach (var moduleFile in moduleFiles) { var fileName = moduleFile.FileName; - ModuleRulePaths.Add(fileName.Substring(0, fileName.Length - ICppProject.ModuleDefineExtension.Length), moduleFile); + var moduleName = fileName.Substring(0, fileName.Length - ICppProject.ModuleDefineExtension.Length); + // Two rule files claiming one module name cannot both win, and the rule assembly would + // fail to compile later with a duplicate-type error that names neither file. Packages + // make this collision far more likely, so say exactly which files clash. + if (ModuleRulePaths.TryGetValue(moduleName, out var existing)) + { + throw new Exception( + $"module \"{moduleName}\" is defined twice:{Environment.NewLine}" + + $" {existing}{Environment.NewLine}" + + $" {moduleFile}"); + } + ModuleRulePaths.Add(moduleName, moduleFile); } var compiler = ServiceContext.Instance.FindService().Value; @@ -160,9 +183,33 @@ class ${moduleName} public void Parse() { + RestorePackages(); ParseRules(); } + /// + /// Brings the declared packages onto disk. This has to happen before : + /// a package's own .module.cs files are globbed into the same rule assembly as the + /// project's, and that assembly is loaded once with Assembly.LoadFile and can never be + /// unloaded - there is no second chance to add rules after the fact. + /// + /// A project without an RBTPackage.json pays nothing: the service returns immediately + /// and no Packages/ directory or lock file is created. + /// + public void RestorePackages() + { + var service = ServiceContext.Instance.FindService(); + if (!service) + { + // Package support is optional; a context that did not register it still builds. + return; + } + + var result = service.Value.Restore(ProjectRoot, PackageArgs.Get().ToRestoreOptions()); + RestoredPackages.Clear(); + RestoredPackages.AddRange(result.Packages); + } + public void Setup() { // generate dll && do init functions @@ -532,6 +579,9 @@ private void PostCompile(CppBuilder builder) private Dictionary TargetRulePaths { get; } = new(); private Dictionary ModuleRulePaths { get; } = new(); + + /// Packages materialized by the last , in dependency order. + private List RestoredPackages { get; } = new(); private IAssemblyCompileUnit BuildRuleCompileUnit { get; set; } private NPath CppBuildRuleProjectOutput => IntermediaFolder.Combine("CppBuildRule/Project"); diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Project/PackageArgs.cs b/ReBuildTool/ReBuildTool.CppCompiler/Project/PackageArgs.cs new file mode 100644 index 0000000..2be60ba --- /dev/null +++ b/ReBuildTool/ReBuildTool.CppCompiler/Project/PackageArgs.cs @@ -0,0 +1,35 @@ +using ReBuildTool.Service.PackageService; +using ResetCore.Common; + +namespace ReBuildTool.CppCompiler; + +/// +/// Command line control over package restore. +/// +/// This group lives in ReBuildTool.CppCompiler rather than next to the package service in +/// ReBuildTool.Service on purpose: CmdParser discovers argument groups by scanning +/// AppDomain.CurrentDomain.GetAssemblies(), and .NET loads assemblies lazily - a group in +/// an assembly nothing has touched yet would simply not be found. CppCompiler is already loaded +/// by the time arguments are parsed, as CppCompilerArgs itself proves. +/// +public class PackageArgs : CommandLineArgGroup +{ + [CmdLine("never access the network during package restore; fail if the lock is not already satisfied")] + public CmdLineArg Offline { get; set; } = CmdLineArg.FromObject(nameof(Offline), false); + + [CmdLine("re-fetch every package even when the lock is already satisfied")] + public CmdLineArg ForceRestore { get; set; } = CmdLineArg.FromObject(nameof(ForceRestore), false); + + [CmdLine("re-resolve moving pins (tags and branches) and rewrite RBTPackage.lock.json")] + public CmdLineArg UpdateLock { get; set; } = CmdLineArg.FromObject(nameof(UpdateLock), false); + + public PackageRestoreOptions ToRestoreOptions() + { + return new PackageRestoreOptions + { + Offline = Offline.Value, + Force = ForceRestore.Value, + UpdateLock = UpdateLock.Value + }; + } +} diff --git a/ReBuildTool/ReBuildTool.Service/CommandGroup/ICommonCommandGroup.cs b/ReBuildTool/ReBuildTool.Service/CommandGroup/ICommonCommandGroup.cs index da23f84..ce03f3f 100644 --- a/ReBuildTool/ReBuildTool.Service/CommandGroup/ICommonCommandGroup.cs +++ b/ReBuildTool/ReBuildTool.Service/CommandGroup/ICommonCommandGroup.cs @@ -8,7 +8,14 @@ public enum RunMode Init, Build, Clean, - ReBuild + ReBuild, + + /// + /// Fetch the packages declared in RBTPackage.json and write the lock, without building. + /// Every other mode restores implicitly, so this is for populating a checkout up front + /// (a CI cache-warm step, or an offline machine's last online moment). + /// + Restore } public interface ICommonCommandGroup : ICommandLineArgGroup diff --git a/ReBuildTool/ReBuildTool.Service/CompileService/CppCompile.cs b/ReBuildTool/ReBuildTool.Service/CompileService/CppCompile.cs index da3a662..5f58326 100644 --- a/ReBuildTool/ReBuildTool.Service/CompileService/CppCompile.cs +++ b/ReBuildTool/ReBuildTool.Service/CompileService/CppCompile.cs @@ -80,6 +80,12 @@ public interface ITargetCompilePlugin } +/// +/// Superseded by the RBTPackage.json / RBTPackage.lock.json package manifest. +/// +[Obsolete("Declare dependencies in RBTPackage.json instead. GitLibraries is never read: it hangs " + + "off a target rule, which only exists once the rule assembly has been compiled, whereas a " + + "package's own .module.cs has to be on disk before that compile. See Doc/HowToUse.md.")] public class GitLibrary { public string Name { get; set; } @@ -97,6 +103,7 @@ public interface ITargetInterface public Dictionary CustomInfo { get; } + [Obsolete("Declare dependencies in RBTPackage.json instead - this list is never read.")] public List GitLibraries { get; } } diff --git a/ReBuildTool/ReBuildTool.Service/Context/ServiceContext.Default.cs b/ReBuildTool/ReBuildTool.Service/Context/ServiceContext.Default.cs index 0d77a9b..7f74832 100644 --- a/ReBuildTool/ReBuildTool.Service/Context/ServiceContext.Default.cs +++ b/ReBuildTool/ReBuildTool.Service/Context/ServiceContext.Default.cs @@ -5,6 +5,7 @@ using ReBuildTool.Service.IDEService.CMake; using ReBuildTool.Service.IDEService.VisualStudio; using ReBuildTool.Service.IDEService.VSCode; +using ReBuildTool.Service.PackageService; namespace ReBuildTool.Service.Context; @@ -21,6 +22,9 @@ public void InitByDefault() var cppDll = FindAssembly("ReBuildTool.CppCompiler"); RegisterType(cppDll, "ReBuildTool.ToolChain.Project.CppBuildProject"); + // Lives in this assembly, so it is registered directly rather than looked up by type name. + RegisterService(new PackageRestoreService()); + var csharpDll = FindAssembly("ReBuildTool.CSharpCompiler"); RegisterType(csharpDll, "ReBuildTool.CSharpCompiler.SimpleAssemblyCompileUnit"); RegisterService(csharpDll, "ReBuildTool.CSharpCompiler.SimpleCompiler"); diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs new file mode 100644 index 0000000..6023af5 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs @@ -0,0 +1,147 @@ +using NiceIO; +using ResetCore.Common; + +namespace ReBuildTool.Service.PackageService.Fetchers; + +/// +/// Clones a git dependency into <ProjectRoot>/Packages/<name> and parks it at an +/// exact commit. +/// +/// The clone keeps its .git directory so a later restore updates with a fetch instead of +/// re-downloading, and so a moving pin (tag or branch) can be re-resolved on --UpdateLock. +/// Whatever the pin was written as, the lock always records the commit it resolved to - tags get +/// moved upstream, commits do not. +/// +public class GitPackageFetcher : IPackageFetcher +{ + public PackageSourceKind Kind => PackageSourceKind.Git; + + public FetchedPackage Fetch(FetchRequest request) + { + var url = request.Dependency.Git!; + var destination = request.DefaultDestination; + var isClone = destination.Combine(".git").DirectoryExists(); + + if (!isClone) + { + // A leftover directory that is not a clone (an interrupted fetch, or a rename) would + // make every git command below fail with a confusing message. Start clean instead. + if (destination.DirectoryExists()) + { + destination.DeleteIfExists(DeleteMode.Normal); + } + RequireNetwork(request, $"package \"{request.Name}\" has not been cloned yet"); + Log.Info($"[package] cloning {request.Name} from {url}"); + destination.EnsureParentDirectoryExists(); + ProcessRunner.RunOrThrow( + "git", + new[] { "clone", "--recurse-submodules", url, destination.ToString() }, + null, + $"cloning package \"{request.Name}\""); + } + + var revision = ResolveRevision(request, destination); + + // Restore runs before every build, so the already-correct case has to be free. Checking + // HEAD costs one git call; resetting unconditionally would rewrite the whole work tree + // (and bump every source file's timestamp, forcing a full recompile) on each build. + if (TryResolve(destination, "HEAD") != revision) + { + ProcessRunner.RunOrThrow( + "git", + new[] { "reset", "--hard", revision }, + destination, + $"checking out package \"{request.Name}\" at {revision}"); + ProcessRunner.RunOrThrow( + "git", + new[] { "submodule", "update", "--init", "--recursive" }, + destination, + $"updating submodules of package \"{request.Name}\""); + } + + return new FetchedPackage(destination, revision); + } + + /// + /// Turns the declared pin into a concrete commit sha, fetching from the remote only when the + /// answer is not already available locally (or when the caller asked to re-resolve). + /// + private string ResolveRevision(FetchRequest request, NPath repository) + { + var dependency = request.Dependency; + + // An explicit commit is already exact; it just has to be present in the clone. + if (!string.IsNullOrWhiteSpace(dependency.Commit)) + { + var local = TryResolve(repository, dependency.Commit); + if (local != null) + { + return local; + } + RequireNetwork(request, $"commit {dependency.Commit} of \"{request.Name}\" is not in the local clone"); + FetchRemote(request, repository); + return TryResolve(repository, dependency.Commit) + ?? throw new PackageException( + $"package \"{request.Name}\": commit {dependency.Commit} does not exist in {dependency.Git}."); + } + + // A tag or a branch is a moving target. Re-resolve it when asked to, otherwise reuse what + // the lock already pinned so a plain build stays reproducible and offline. + var reference = !string.IsNullOrWhiteSpace(dependency.Tag) + ? $"refs/tags/{dependency.Tag}" + : $"refs/remotes/origin/{dependency.Branch}"; + + if (!request.Options.UpdateLock && request.Locked?.Resolved != null) + { + var pinned = TryResolve(repository, request.Locked.Resolved); + if (pinned != null) + { + return pinned; + } + } + + if (request.Options.UpdateLock || TryResolve(repository, reference) == null) + { + RequireNetwork(request, $"\"{request.Name}\" needs {reference} resolved against the remote"); + FetchRemote(request, repository); + } + + return TryResolve(repository, reference) + ?? throw new PackageException( + $"package \"{request.Name}\": {reference} does not exist in {dependency.Git}."); + } + + private static void FetchRemote(FetchRequest request, NPath repository) + { + Log.Info($"[package] fetching {request.Name}"); + ProcessRunner.RunOrThrow( + "git", + new[] { "fetch", "--tags", "--force", "origin" }, + repository, + $"fetching package \"{request.Name}\""); + } + + /// Resolves a revision to a commit sha, or null when git does not know it locally. + private static string? TryResolve(NPath repository, string reference) + { + var result = ProcessRunner.Run( + "git", + new[] { "rev-parse", "--verify", "--quiet", $"{reference}^{{commit}}" }, + repository); + if (!result.IsSuccess) + { + return null; + } + var sha = result.StdOut.Trim(); + return string.IsNullOrEmpty(sha) ? null : sha; + } + + private static void RequireNetwork(FetchRequest request, string why) + { + if (request.Options.Offline) + { + throw new PackageException( + $"--Offline was requested but {why}. Run a restore without --Offline first."); + } + } +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs new file mode 100644 index 0000000..9ff1942 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs @@ -0,0 +1,61 @@ +using NiceIO; + +namespace ReBuildTool.Service.PackageService.Fetchers; + +public class FetchRequest +{ + public FetchRequest( + string name, + PackageDependency dependency, + NPath declaringDirectory, + NPath packagesRoot, + PackageRestoreOptions options, + LockedPackage? locked) + { + Name = name; + Dependency = dependency; + DeclaringDirectory = declaringDirectory; + PackagesRoot = packagesRoot; + Options = options; + Locked = locked; + } + + public string Name { get; } + + public PackageDependency Dependency { get; } + + /// Directory of the manifest that declared this dependency - relative paths resolve against it. + public NPath DeclaringDirectory { get; } + + /// <ProjectRoot>/Packages. + public NPath PackagesRoot { get; } + + public PackageRestoreOptions Options { get; } + + /// The matching lock entry, when the project already has one. + public LockedPackage? Locked { get; } + + public NPath DefaultDestination => PackagesRoot.Combine(Name); +} + +public class FetchedPackage +{ + public FetchedPackage(NPath root, string resolved) + { + Root = root; + Resolved = resolved; + } + + /// Where the package content lives. For a path dependency this is outside Packages/. + public NPath Root { get; } + + /// What the pin actually resolved to: a commit sha, an archive hash, or an absolute path. + public string Resolved { get; } +} + +public interface IPackageFetcher +{ + PackageSourceKind Kind { get; } + + FetchedPackage Fetch(FetchRequest request); +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/PathPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/PathPackageFetcher.cs new file mode 100644 index 0000000..e30166e --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/PathPackageFetcher.cs @@ -0,0 +1,39 @@ +using NiceIO; + +namespace ReBuildTool.Service.PackageService.Fetchers; + +/// +/// A dependency on a directory already present on this machine. +/// +/// Nothing is copied: the package is used where it lies, so edits in the depended-on source show +/// up in the very next build. That is the point of a path dependency - local co-development of a +/// library and its consumer. +/// +public class PathPackageFetcher : IPackageFetcher +{ + public PackageSourceKind Kind => PackageSourceKind.Path; + + public FetchedPackage Fetch(FetchRequest request) + { + var declared = request.Dependency.Path!; + var resolved = System.IO.Path.IsPathRooted(declared) + ? declared.ToNPath() + : request.DeclaringDirectory.Combine(declared); + + // MakeAbsolute collapses the ".." that a sibling-directory dependency almost always uses, + // so the lock records a stable path rather than one relative to the declaring manifest. + resolved = resolved.MakeAbsolute(); + + if (!resolved.DirectoryExists()) + { + throw new PackageException( + $"path dependency \"{request.Name}\" points at \"{declared}\", which resolves to " + + $"\"{resolved}\" - that directory does not exist."); + } + + // The lock records the path as it was declared, not where it landed on this machine: an + // absolute path is derivable from the manifest anyway, and committing one would make the + // lock file useless to every other checkout. + return new FetchedPackage(resolved, declared); + } +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/IPackageService.cs b/ReBuildTool/ReBuildTool.Service/PackageService/IPackageService.cs new file mode 100644 index 0000000..8795165 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/IPackageService.cs @@ -0,0 +1,62 @@ +using NiceIO; +using ReBuildTool.Service.Context; + +namespace ReBuildTool.Service.PackageService; + +public class PackageRestoreOptions +{ + /// Never touch the network: if the lock is not already satisfied on disk, fail. + public bool Offline { get; set; } + + /// Re-fetch every package even when the lock is satisfied. + public bool Force { get; set; } + + /// Re-resolve moving refs (tags/branches) and rewrite the lock, like cargo update. + public bool UpdateLock { get; set; } +} + +/// +/// A package that has been materialized on disk and is ready to take part in the build. +/// +public class RestoredPackage +{ + public RestoredPackage(string name, NPath root, PackageManifest? manifest) + { + Name = name; + Root = root; + Manifest = manifest; + } + + public string Name { get; } + + /// Where the package's content lives - the directory rbt globs rule files out of. + public NPath Root { get; } + + public PackageManifest? Manifest { get; } +} + +public class PackageRestoreResult +{ + public static PackageRestoreResult Empty { get; } = new(new List()); + + public PackageRestoreResult(List packages) + { + Packages = packages; + } + + public List Packages { get; } +} + +/// +/// Fetches every package the project's RBTPackage.json transitively depends on and +/// materializes it under <ProjectRoot>/Packages/. +/// +/// This has to run before the rule files are globbed and compiled: a package's own +/// .module.cs must already be on disk when CppBuildProject.ParseRules builds the +/// CompileRules.dll compile unit, because that assembly is loaded exactly once with +/// Assembly.LoadFile and cannot be unloaded and rebuilt afterwards. +/// +public interface IPackageService : IService +{ + PackageRestoreResult Restore(NPath projectRoot, PackageRestoreOptions options); +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageLock.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageLock.cs new file mode 100644 index 0000000..c96a983 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageLock.cs @@ -0,0 +1,102 @@ +using NiceIO; +using Newtonsoft.Json; + +namespace ReBuildTool.Service.PackageService; + +/// +/// One resolved package as recorded in the lock file. is what makes a +/// restore reproducible: for git it is the commit a tag or branch actually pointed at, which +/// upstream is free to move afterwards. +/// +public class LockedPackage +{ + [JsonProperty("name")] public string Name { get; set; } = string.Empty; + + [JsonProperty("source")] public string Source { get; set; } = string.Empty; + + /// The git/http URL, or the declared path for a path dependency. + [JsonProperty("origin")] public string? Origin { get; set; } + + /// Commit sha for git, content sha256 for an archive, the absolute path for a path dependency. + [JsonProperty("resolved")] public string? Resolved { get; set; } + + /// The pin this entry was produced from, so a changed manifest invalidates the lock. + [JsonProperty("pin")] public string? Pin { get; set; } + + [JsonProperty("dependencies")] public List Dependencies { get; set; } = new(); +} + +/// +/// RBTPackage.lock.json - the resolver's output, and the reason a second restore can skip +/// the network entirely. Meant to be committed to version control. +/// +public class PackageLockFile +{ + public const string FileName = "RBTPackage.lock.json"; + + public const int CurrentVersion = 1; + + [JsonProperty("version")] public int Version { get; set; } = CurrentVersion; + + [JsonProperty("packages")] public List Packages { get; set; } = new(); + + public static NPath PathIn(NPath projectRoot) => projectRoot.Combine(FileName); + + public static PackageLockFile? ReadFrom(NPath projectRoot) + { + var path = PathIn(projectRoot); + if (!path.FileExists()) + { + return null; + } + + PackageLockFile? lockFile; + try + { + lockFile = JsonConvert.DeserializeObject(path.ReadAllText()); + } + catch (JsonException e) + { + throw new PackageException($"{path} is not valid JSON: {e.Message}", e); + } + + if (lockFile == null) + { + return null; + } + lockFile.Packages ??= new List(); + + // A lock written by a newer rbt may use fields this build does not understand. Re-resolving + // is always correct, so treat it as absent rather than failing the build. + if (lockFile.Version != CurrentVersion) + { + return null; + } + return lockFile; + } + + public LockedPackage? Find(string name) + { + return Packages.FirstOrDefault(package => package.Name == name); + } + + /// + /// Writes the lock only when its content actually changed. Rewriting it unconditionally would + /// bump the file's timestamp on every single build, and rbt's incremental checks + /// (NeedReBuildRuleAssembly, the makefile backend) are timestamp based - see the same + /// reasoning in CppModuleRule.GenerateCode. + /// + public void WriteIfChanged(NPath projectRoot) + { + // Stable ordering keeps the file diff-friendly across machines. + Packages = Packages.OrderBy(package => package.Name, StringComparer.Ordinal).ToList(); + var path = PathIn(projectRoot); + var content = JsonConvert.SerializeObject(this, Formatting.Indented) + Environment.NewLine; + if (path.FileExists() && path.ReadAllText() == content) + { + return; + } + path.EnsureParentDirectoryExists(); + path.WriteAllText(content); + } +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs new file mode 100644 index 0000000..008f7e9 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs @@ -0,0 +1,226 @@ +using NiceIO; +using Newtonsoft.Json; + +namespace ReBuildTool.Service.PackageService; + +/// +/// Where a package's content comes from. Exactly one of the corresponding fields on +/// may be set. +/// +public enum PackageSourceKind +{ + Git, + HttpArchive, + Path, + Vcpkg +} + +/// +/// One entry of a manifest's dependencies map. All source fields are optional and +/// mutually exclusive - validates that exactly one is set. +/// +public class PackageDependency +{ + [JsonProperty("git")] public string? Git { get; set; } + + [JsonProperty("tag")] public string? Tag { get; set; } + + [JsonProperty("branch")] public string? Branch { get; set; } + + [JsonProperty("commit")] public string? Commit { get; set; } + + [JsonProperty("path")] public string? Path { get; set; } + + [JsonProperty("url")] public string? Url { get; set; } + + [JsonProperty("sha256")] public string? Sha256 { get; set; } + + /// + /// Leading path components to strip when extracting an archive, like tar --strip-components. + /// Upstream release tarballs almost always wrap everything in a single name-version/ + /// directory, so 1 is the common value. + /// + [JsonProperty("strip")] public int Strip { get; set; } + + [JsonProperty("vcpkg")] public string? Vcpkg { get; set; } + + [JsonProperty("version")] public string? Version { get; set; } + + /// + /// Path (relative to the manifest that declares this dependency) of a .module.cs to + /// copy into the fetched package. For upstream sources that ship no rbt rule of their own. + /// + [JsonProperty("overlay")] public string? Overlay { get; set; } + + public PackageSourceKind ResolveKind(string packageName) + { + var kinds = new List(); + if (!string.IsNullOrWhiteSpace(Git)) + { + kinds.Add(PackageSourceKind.Git); + } + if (!string.IsNullOrWhiteSpace(Url)) + { + kinds.Add(PackageSourceKind.HttpArchive); + } + if (!string.IsNullOrWhiteSpace(Path)) + { + kinds.Add(PackageSourceKind.Path); + } + if (!string.IsNullOrWhiteSpace(Vcpkg)) + { + kinds.Add(PackageSourceKind.Vcpkg); + } + + if (kinds.Count == 0) + { + throw new PackageException( + $"package \"{packageName}\" declares no source: set exactly one of " + + $"\"git\", \"url\", \"path\" or \"vcpkg\"."); + } + if (kinds.Count > 1) + { + throw new PackageException( + $"package \"{packageName}\" declares more than one source ({string.Join(", ", kinds)}): " + + $"set exactly one of \"git\", \"url\", \"path\" or \"vcpkg\"."); + } + + if (kinds[0] == PackageSourceKind.Git && Commit == null && Tag == null && Branch == null) + { + throw new PackageException( + $"package \"{packageName}\" pins no git revision: set \"commit\", \"tag\" or \"branch\". " + + $"rbt resolves exact pins only - it never picks a version for you."); + } + + return kinds[0]; + } + + /// + /// The git revision to check out, most specific first. A commit is reproducible, a tag can be + /// moved upstream, a branch moves constantly - but the lock file always records the commit + /// each of them actually resolved to. + /// + public string? GitRevision => Commit ?? Tag ?? Branch; + + /// + /// Identity of this pin, used to detect conflicting declarations of the same package name + /// coming from different manifests. Two dependencies with the same key are interchangeable. + /// + public string PinKey(string packageName) + { + return ResolveKind(packageName) switch + { + PackageSourceKind.Git => $"git:{Git}@{GitRevision}", + PackageSourceKind.HttpArchive => $"url:{Url}@{Sha256}", + PackageSourceKind.Path => $"path:{Path}", + PackageSourceKind.Vcpkg => $"vcpkg:{Vcpkg}@{Version}", + _ => throw new PackageException($"unknown source kind for package \"{packageName}\"") + }; + } + + public string Describe(string packageName) => PinKey(packageName); +} + +/// +/// Prebuilt artifacts shipped by a binary package, declared in the package's own manifest. +/// Platform / architecture / configuration are matched as strings because the enums that +/// name them (PlatformSupportType, BuildConfiguration, Architecture) +/// live in ReBuildTool.CppCompiler, which this assembly sits below. +/// +public class PackageBinarySpec +{ + [JsonProperty("includes")] public List Includes { get; set; } = new(); + + [JsonProperty("artifacts")] public List Artifacts { get; set; } = new(); +} + +public class PackageBinaryArtifact +{ + /// Matches PlatformSupportType by name (Windows, Linux, MacOSX, ...). Null matches every platform. + [JsonProperty("platform")] public string? Platform { get; set; } + + /// Matches Architecture.CommandLineName (x86, x64, arm32, arm64). Null matches every architecture. + [JsonProperty("arch")] public string? Arch { get; set; } + + /// Matches BuildConfiguration by name (Debug, Release, ...). Null matches every configuration. + [JsonProperty("config")] public string? Config { get; set; } + + [JsonProperty("libraryDirectories")] public List LibraryDirectories { get; set; } = new(); + + [JsonProperty("staticLibraries")] public List StaticLibraries { get; set; } = new(); + + [JsonProperty("dynamicLibraries")] public List DynamicLibraries { get; set; } = new(); + + [JsonProperty("defines")] public List Defines { get; set; } = new(); +} + +/// +/// A project's or a package's RBTPackage.json. A package declares its own transitive +/// dependencies with the very same file, which is what lets the resolver walk the graph. +/// +public class PackageManifest +{ + public const string FileName = "RBTPackage.json"; + + [JsonProperty("name")] public string? Name { get; set; } + + [JsonProperty("dependencies")] public Dictionary Dependencies { get; set; } = new(); + + /// + /// Root-manifest-only escape hatch: when two packages pin the same dependency differently the + /// resolver refuses to guess, and the user names the winning pin here. + /// + [JsonProperty("overrides")] public Dictionary Overrides { get; set; } = new(); + + [JsonProperty("binary")] public PackageBinarySpec? Binary { get; set; } + + public static NPath PathIn(NPath directory) => directory.Combine(FileName); + + /// + /// Reads the manifest sitting in , or null when there is none - + /// a package without a manifest is legal, it simply has no transitive dependencies. + /// + public static PackageManifest? ReadFrom(NPath directory) + { + var path = PathIn(directory); + if (!path.FileExists()) + { + return null; + } + return Parse(path.ReadAllText(), path); + } + + public static PackageManifest Parse(string json, NPath origin) + { + PackageManifest? manifest; + try + { + manifest = JsonConvert.DeserializeObject(json); + } + catch (JsonException e) + { + throw new PackageException($"{origin} is not valid JSON: {e.Message}", e); + } + + if (manifest == null) + { + throw new PackageException($"{origin} is empty."); + } + + // A "dependencies": null in the file deserializes to null rather than the initializer. + manifest.Dependencies ??= new Dictionary(); + manifest.Overrides ??= new Dictionary(); + return manifest; + } +} + +public class PackageException : Exception +{ + public PackageException(string message) : base(message) + { + } + + public PackageException(string message, Exception inner) : base(message, inner) + { + } +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs new file mode 100644 index 0000000..af50e67 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs @@ -0,0 +1,160 @@ +using NiceIO; +using ReBuildTool.Service.PackageService.Fetchers; +using ResetCore.Common; + +namespace ReBuildTool.Service.PackageService; + +/// +/// Walks the dependency graph depth-first, fetching each package and then reading the manifest it +/// brought with it to discover the next level. +/// +/// rbt resolves exact pins only: there is no version-range solving, so two packages that +/// pin the same dependency differently is a hard error the user resolves with an explicit +/// overrides entry. That keeps the algorithm a plain graph walk with no backtracking, and +/// keeps builds reproducible without a solver. +/// +public class PackageResolver +{ + public PackageResolver(NPath packagesRoot, IEnumerable fetchers) + { + PackagesRoot = packagesRoot; + Fetchers = fetchers.ToDictionary(fetcher => fetcher.Kind); + } + + private NPath PackagesRoot { get; } + + private Dictionary Fetchers { get; } + + private Dictionary Resolved { get; } = new(); + + /// The DFS path currently being expanded, used to name the members of a dependency cycle. + private List Visiting { get; } = new(); + + private Dictionary Overrides { get; set; } = new(); + + private PackageLockFile? ExistingLock { get; set; } + + private PackageRestoreOptions Options { get; set; } = new(); + + private class ResolvedEntry + { + public required string PinKey { get; init; } + public required RestoredPackage Package { get; init; } + public required LockedPackage Locked { get; init; } + } + + public PackageRestoreResult Resolve( + PackageManifest rootManifest, + NPath rootDirectory, + PackageLockFile? existingLock, + PackageRestoreOptions options, + out PackageLockFile newLock) + { + Resolved.Clear(); + Visiting.Clear(); + Overrides = rootManifest.Overrides; + ExistingLock = existingLock; + Options = options; + + foreach (var (name, dependency) in rootManifest.Dependencies) + { + ResolveOne(name, dependency, rootDirectory); + } + + newLock = new PackageLockFile + { + Packages = Resolved.Values.Select(entry => entry.Locked).ToList() + }; + + // Deepest-first: a package is listed after everything it needed, which is the order a + // reader wants and costs nothing to produce here. + return new PackageRestoreResult(Resolved.Values.Select(entry => entry.Package).ToList()); + } + + private void ResolveOne(string name, PackageDependency declared, NPath declaringDirectory) + { + var dependency = Overrides.TryGetValue(name, out var overridden) ? overridden : declared; + var pinKey = dependency.PinKey(name); + + // Cycle check first: a package is recorded in Resolved before its own dependencies are + // walked (so that a diamond is fetched once), which means an ancestor looks "already + // resolved" too. Only Visiting distinguishes "seen before" from "currently on the stack". + if (Visiting.Contains(name)) + { + var cycle = string.Join(" -> ", Visiting.Concat(new[] { name })); + throw new PackageException($"dependency cycle between packages: {cycle}"); + } + + if (Resolved.TryGetValue(name, out var already)) + { + if (already.PinKey != pinKey) + { + throw new PackageException( + $"conflicting pins for package \"{name}\":{Environment.NewLine}" + + $" {already.PinKey}{Environment.NewLine}" + + $" {pinKey}{Environment.NewLine}" + + $"rbt does not pick a version for you. Add an \"overrides\" entry for \"{name}\" " + + $"in the project's {PackageManifest.FileName} to say which one wins."); + } + return; + } + + var kind = dependency.ResolveKind(name); + if (!Fetchers.TryGetValue(kind, out var fetcher)) + { + throw new PackageException( + $"package \"{name}\" uses the {kind} source, which this build of rbt cannot fetch yet."); + } + + Visiting.Add(name); + try + { + var request = new FetchRequest( + name, + dependency, + declaringDirectory, + PackagesRoot, + Options, + ExistingLock?.Find(name)); + var fetched = fetcher.Fetch(request); + var manifest = PackageManifest.ReadFrom(fetched.Root); + + // Recorded before descending so a cycle back to this package is caught by Visiting + // rather than by re-fetching. + var locked = new LockedPackage + { + Name = name, + Source = kind.ToString(), + Origin = dependency.Git ?? dependency.Url ?? dependency.Path ?? dependency.Vcpkg, + Resolved = fetched.Resolved, + Pin = pinKey, + Dependencies = manifest?.Dependencies.Keys.OrderBy(key => key, StringComparer.Ordinal).ToList() + ?? new List() + }; + Resolved[name] = new ResolvedEntry + { + PinKey = pinKey, + Package = new RestoredPackage(name, fetched.Root, manifest), + Locked = locked + }; + + if (manifest != null) + { + foreach (var (childName, childDependency) in manifest.Dependencies) + { + if (childName == name) + { + throw new PackageException($"package \"{name}\" depends on itself."); + } + ResolveOne(childName, childDependency, fetched.Root); + } + } + } + finally + { + Visiting.Remove(name); + } + + Log.Info($"[package] {name} -> {Resolved[name].Locked.Resolved}"); + } +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs new file mode 100644 index 0000000..69cb040 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs @@ -0,0 +1,87 @@ +using NiceIO; +using ReBuildTool.Service.PackageService.Fetchers; +using ResetCore.Common; + +namespace ReBuildTool.Service.PackageService; + +/// +/// The default : reads the project manifest, resolves and fetches the +/// whole dependency graph into <ProjectRoot>/Packages/, and writes the lock. +/// +/// Packages deliberately do not live under Intermedia/: CppBuildProject.Clean +/// empties that directory, and CleanIfNeed triggers a clean on its own whenever the rbt +/// binaries are newer than the last build - dependencies would be re-downloaded after every +/// rebuild and after every rbt update. +/// +public class PackageRestoreService : IPackageService +{ + public const string PackagesFolderName = "Packages"; + + public PackageRestoreResult Restore(NPath projectRoot, PackageRestoreOptions options) + { + var manifestPath = PackageManifest.PathIn(projectRoot); + if (!manifestPath.FileExists()) + { + // No manifest means no package management at all: a project that does not use the + // feature must not get a Packages/ directory, a lock file or a .gitignore edit. + return PackageRestoreResult.Empty; + } + + var manifest = PackageManifest.Parse(manifestPath.ReadAllText(), manifestPath); + if (manifest.Dependencies.Count == 0) + { + return PackageRestoreResult.Empty; + } + + var packagesRoot = projectRoot.Combine(PackagesFolderName); + packagesRoot.EnsureDirectoryExists(); + EnsureGitIgnored(projectRoot); + + var existingLock = options.Force ? null : PackageLockFile.ReadFrom(projectRoot); + var resolver = new PackageResolver(packagesRoot, CreateFetchers()); + var result = resolver.Resolve(manifest, projectRoot, existingLock, options, out var newLock); + newLock.WriteIfChanged(projectRoot); + + return result; + } + + private static IEnumerable CreateFetchers() + { + yield return new GitPackageFetcher(); + yield return new PathPackageFetcher(); + } + + /// + /// Keeps the materialized Packages/ tree out of the consuming repository. Only touches a + /// .gitignore that already exists or a directory that is actually a git repository, and only + /// when the pattern is not already there, so it stays a no-op on every subsequent build. + /// + private static void EnsureGitIgnored(NPath projectRoot) + { + var ignorePath = projectRoot.Combine(".gitignore"); + if (!ignorePath.FileExists() && !projectRoot.Combine(".git").DirectoryExists()) + { + return; + } + + var pattern = $"/{PackagesFolderName}/"; + var lines = ignorePath.FileExists() + ? ignorePath.ReadAllLines().ToList() + : new List(); + if (lines.Any(line => line.Trim() == pattern || line.Trim() == PackagesFolderName)) + { + return; + } + + lines.Add(pattern); + try + { + ignorePath.WriteAllLines(lines.ToArray()); + } + catch (Exception e) + { + // Never fail a build over a convenience edit to a file rbt does not own. + Log.Warning($"[package] could not add \"{pattern}\" to {ignorePath}: {e.Message}"); + } + } +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs b/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs new file mode 100644 index 0000000..97b95e5 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs @@ -0,0 +1,105 @@ +using System.Diagnostics; +using System.Text; +using NiceIO; + +namespace ReBuildTool.Service.PackageService; + +public class ProcessResult +{ + public int ExitCode { get; init; } + + public string StdOut { get; init; } = string.Empty; + + public string StdErr { get; init; } = string.Empty; + + public bool IsSuccess => ExitCode == 0; +} + +/// +/// Runs an external tool and captures its output. +/// +/// The package layer cannot use ReBuildTool.Service.Global.Shell for this: that wrapper +/// forwards stdout/stderr straight into the logger and keeps nothing, while resolving a git pin +/// means reading git rev-parse HEAD back. Arguments go through ArgumentList so the +/// runtime applies the OS-correct argv quoting per token, exactly as Shell does. +/// +internal static class ProcessRunner +{ + public static ProcessResult Run(string program, IEnumerable arguments, NPath? workingDirectory = null) + { + var startInfo = new ProcessStartInfo + { + FileName = program, + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true + }; + foreach (var argument in arguments) + { + startInfo.ArgumentList.Add(argument); + } + if (workingDirectory != null && workingDirectory.DirectoryExists()) + { + startInfo.WorkingDirectory = workingDirectory.ToString(); + } + + using var process = new Process(); + process.StartInfo = startInfo; + + var stdOut = new StringBuilder(); + var stdErr = new StringBuilder(); + process.OutputDataReceived += (_, args) => + { + if (args.Data != null) + { + stdOut.AppendLine(args.Data); + } + }; + process.ErrorDataReceived += (_, args) => + { + if (args.Data != null) + { + stdErr.AppendLine(args.Data); + } + }; + + try + { + process.Start(); + } + catch (Exception e) + { + throw new PackageException($"cannot run \"{program}\": {e.Message}", e); + } + + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + process.WaitForExit(); + + return new ProcessResult + { + ExitCode = process.ExitCode, + StdOut = stdOut.ToString(), + StdErr = stdErr.ToString() + }; + } + + /// Runs the tool and returns its trimmed stdout, throwing with the captured stderr on failure. + public static string RunOrThrow( + string program, + IEnumerable arguments, + NPath? workingDirectory, + string what) + { + var argumentList = arguments.ToList(); + var result = Run(program, argumentList, workingDirectory); + if (!result.IsSuccess) + { + var details = string.IsNullOrWhiteSpace(result.StdErr) ? result.StdOut : result.StdErr; + throw new PackageException( + $"{what} failed (exit {result.ExitCode}): {program} {string.Join(" ", argumentList)}{Environment.NewLine}{details.Trim()}"); + } + return result.StdOut.Trim(); + } +} diff --git a/ReBuildTool/ReBuildTool.Test/TestCppBuild.cs b/ReBuildTool/ReBuildTool.Test/TestCppBuild.cs index efd1438..be44e07 100644 --- a/ReBuildTool/ReBuildTool.Test/TestCppBuild.cs +++ b/ReBuildTool/ReBuildTool.Test/TestCppBuild.cs @@ -24,11 +24,14 @@ public void Setup() } // One subdirectory under Sample/ per compile scenario: plain executable (BuildCpp), - // static-library linking, dynamic-library linking, and a three-level module chain. + // static-library linking, dynamic-library linking, a three-level module chain, and a project + // whose dependency arrives through package restore (PackageConsumer -> Sample/GeometryPackage, + // a path dependency so the case stays offline and deterministic on every CI host). [TestCase("BuildCpp")] [TestCase("StaticLibraryLink")] [TestCase("DynamicLibraryLink")] [TestCase("MultiModuleChain")] + [TestCase("PackageConsumer")] public void TestSampleProjectBuild(string sampleName) { CmdParser.Parse(); diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageManifest.cs b/ReBuildTool/ReBuildTool.Test/TestPackageManifest.cs new file mode 100644 index 0000000..d1a82e5 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Test/TestPackageManifest.cs @@ -0,0 +1,176 @@ +using NiceIO; +using ReBuildTool.Service.PackageService; + +namespace ReBuildTool.Test; + +/// +/// Manifest validation and lock-file behaviour. A bad manifest has to fail with a message that +/// says what to write instead - these are the errors a user meets first. +/// +[TestFixture] +public class TestPackageManifest +{ + private NPath WorkDirectory = null!; + + [SetUp] + public void SetUp() + { + WorkDirectory = Path.Combine(Path.GetTempPath(), $"rbt-manifest-{Guid.NewGuid():N}") + .ToNPath() + .EnsureDirectoryExists(); + } + + [TearDown] + public void TearDown() + { + WorkDirectory.DeleteIfExists(DeleteMode.Normal); + } + + private static PackageDependency DependencyFrom(string json) + { + return PackageManifest.Parse($"{{ \"dependencies\": {{ \"Some\": {json} }} }}", "test".ToNPath()) + .Dependencies["Some"]; + } + + [Test] + public void ADependencyWithNoSourceIsRejected() + { + var exception = Assert.Throws(() => DependencyFrom("{ }").ResolveKind("Some")); + + Assert.That(exception!.Message, Does.Contain("git")); + Assert.That(exception.Message, Does.Contain("path")); + } + + [Test] + public void ADependencyWithTwoSourcesIsRejected() + { + var dependency = DependencyFrom("{ \"git\": \"https://x/y.git\", \"tag\": \"v1\", \"path\": \"../y\" }"); + + var exception = Assert.Throws(() => dependency.ResolveKind("Some")); + + Assert.That(exception!.Message, Does.Contain("more than one source")); + } + + [Test] + public void AGitDependencyWithoutARevisionIsRejected() + { + // Without a pin the build is not reproducible, and rbt deliberately has no solver to + // choose one - so this must fail loudly rather than silently take the default branch. + var exception = Assert.Throws( + () => DependencyFrom("{ \"git\": \"https://x/y.git\" }").ResolveKind("Some")); + + Assert.That(exception!.Message, Does.Contain("commit")); + Assert.That(exception.Message, Does.Contain("tag")); + } + + [Test] + public void GitRevisionPrefersTheMostSpecificPin() + { + var dependency = DependencyFrom( + "{ \"git\": \"https://x/y.git\", \"commit\": \"abc123\", \"tag\": \"v1\", \"branch\": \"main\" }"); + + Assert.That(dependency.GitRevision, Is.EqualTo("abc123")); + Assert.That(DependencyFrom("{ \"git\": \"https://x/y.git\", \"tag\": \"v1\", \"branch\": \"main\" }").GitRevision, + Is.EqualTo("v1")); + } + + [Test] + public void PinsDifferWhenTheRevisionDiffers() + { + var one = DependencyFrom("{ \"git\": \"https://x/y.git\", \"tag\": \"v1\" }"); + var two = DependencyFrom("{ \"git\": \"https://x/y.git\", \"tag\": \"v2\" }"); + var same = DependencyFrom("{ \"git\": \"https://x/y.git\", \"tag\": \"v1\" }"); + + Assert.That(one.PinKey("Some"), Is.Not.EqualTo(two.PinKey("Some"))); + Assert.That(one.PinKey("Some"), Is.EqualTo(same.PinKey("Some"))); + } + + [Test] + public void AMissingManifestIsNotAnError() + { + // A package is allowed to ship no manifest at all - it simply has no dependencies. + Assert.That(PackageManifest.ReadFrom(WorkDirectory), Is.Null); + } + + [Test] + public void InvalidJsonNamesTheOffendingFile() + { + var path = PackageManifest.PathIn(WorkDirectory); + path.WriteAllText("{ this is not json"); + + var exception = Assert.Throws(() => PackageManifest.ReadFrom(WorkDirectory)); + + Assert.That(exception!.Message, Does.Contain(PackageManifest.FileName)); + } + + [Test] + public void ANullDependencyMapDeserializesToAnEmptyOne() + { + var manifest = PackageManifest.Parse("{ \"dependencies\": null }", "test".ToNPath()); + + Assert.That(manifest.Dependencies, Is.Not.Null); + Assert.That(manifest.Dependencies, Is.Empty); + } + + [Test] + public void TheLockRoundTrips() + { + var original = new PackageLockFile + { + Packages = + { + new LockedPackage + { + Name = "Some", + Source = "Git", + Origin = "https://x/y.git", + Resolved = "abc123", + Pin = "git:https://x/y.git@v1", + Dependencies = { "Other" } + } + } + }; + original.WriteIfChanged(WorkDirectory); + + var reread = PackageLockFile.ReadFrom(WorkDirectory); + + Assert.That(reread, Is.Not.Null); + var package = reread!.Find("Some"); + Assert.That(package, Is.Not.Null); + Assert.That(package!.Resolved, Is.EqualTo("abc123")); + Assert.That(package.Pin, Is.EqualTo("git:https://x/y.git@v1")); + Assert.That(package.Dependencies, Is.EqualTo(new[] { "Other" })); + } + + /// + /// An unchanged lock must not be rewritten. rbt's incremental checks are timestamp based + /// (NeedReBuildRuleAssembly, the makefile backend), so a needless rewrite on every build would + /// keep re-triggering work downstream. + /// + [Test] + public void RewritingAnUnchangedLockDoesNotTouchTheFile() + { + var lockFile = new PackageLockFile + { + Packages = { new LockedPackage { Name = "Some", Source = "Git", Resolved = "abc123" } } + }; + lockFile.WriteIfChanged(WorkDirectory); + var path = PackageLockFile.PathIn(WorkDirectory); + var writtenAt = File.GetLastWriteTimeUtc(path); + + // Coarse filesystem timestamps would hide a rewrite that happened within the same tick. + Thread.Sleep(1100); + PackageLockFile.ReadFrom(WorkDirectory)!.WriteIfChanged(WorkDirectory); + + Assert.That(File.GetLastWriteTimeUtc(path), Is.EqualTo(writtenAt)); + } + + [Test] + public void ALockFromAFutureVersionIsIgnoredRatherThanFatal() + { + PackageLockFile.PathIn(WorkDirectory).WriteAllText("{ \"version\": 999, \"packages\": [] }"); + + // Re-resolving is always correct, so an unreadable lock must not break the build. + Assert.That(PackageLockFile.ReadFrom(WorkDirectory), Is.Null); + } +} diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageResolver.cs b/ReBuildTool/ReBuildTool.Test/TestPackageResolver.cs new file mode 100644 index 0000000..a6f967a --- /dev/null +++ b/ReBuildTool/ReBuildTool.Test/TestPackageResolver.cs @@ -0,0 +1,167 @@ +using NiceIO; +using ReBuildTool.Service.PackageService; +using ReBuildTool.Service.PackageService.Fetchers; + +namespace ReBuildTool.Test; + +/// +/// The dependency graph walk, exercised through real path dependencies on disk. Everything here is +/// offline and deterministic: no network, no git, so it behaves identically on all three CI hosts. +/// +[TestFixture] +public class TestPackageResolver +{ + private NPath WorkDirectory = null!; + + [SetUp] + public void SetUp() + { + WorkDirectory = Path.Combine(Path.GetTempPath(), $"rbt-resolver-{Guid.NewGuid():N}") + .ToNPath() + .EnsureDirectoryExists(); + } + + [TearDown] + public void TearDown() + { + WorkDirectory.DeleteIfExists(DeleteMode.Normal); + } + + /// Writes a package directory containing a manifest that path-depends on the given names. + private NPath WritePackage(string name, params string[] dependsOn) + { + var directory = WorkDirectory.Combine(name).EnsureDirectoryExists(); + var entries = dependsOn.Select(dependency => $"\"{dependency}\": {{ \"path\": \"../{dependency}\" }}"); + PackageManifest.PathIn(directory).WriteAllText( + $"{{ \"name\": \"{name}\", \"dependencies\": {{ {string.Join(", ", entries)} }} }}"); + return directory; + } + + private PackageRestoreResult Resolve(PackageManifest root, NPath rootDirectory) + { + var resolver = new PackageResolver( + WorkDirectory.Combine("Packages"), + new IPackageFetcher[] { new PathPackageFetcher() }); + return resolver.Resolve(root, rootDirectory, null, new PackageRestoreOptions(), out _); + } + + private PackageManifest RootDependingOn(params string[] names) + { + var entries = names.Select(name => $"\"{name}\": {{ \"path\": \"../{name}\" }}"); + return PackageManifest.Parse( + $"{{ \"name\": \"Root\", \"dependencies\": {{ {string.Join(", ", entries)} }} }}", + WorkDirectory.Combine("root", PackageManifest.FileName)); + } + + [Test] + public void TransitiveDependenciesAreResolved() + { + WritePackage("A", "B"); + WritePackage("B", "C"); + WritePackage("C"); + var rootDirectory = WorkDirectory.Combine("root").EnsureDirectoryExists(); + + var result = Resolve(RootDependingOn("A"), rootDirectory); + + // B and C are never named by the root - they are only reachable by reading A's and B's + // own manifests, which is the whole point of the walk. + Assert.That( + result.Packages.Select(package => package.Name).OrderBy(name => name), + Is.EqualTo(new[] { "A", "B", "C" })); + } + + [Test] + public void DiamondDependencyIsFetchedOnce() + { + WritePackage("Left", "Shared"); + WritePackage("Right", "Shared"); + WritePackage("Shared"); + var rootDirectory = WorkDirectory.Combine("root").EnsureDirectoryExists(); + + var result = Resolve(RootDependingOn("Left", "Right"), rootDirectory); + + Assert.That(result.Packages.Count(package => package.Name == "Shared"), Is.EqualTo(1)); + } + + [Test] + public void DependencyCycleIsReportedWithTheWholeChain() + { + WritePackage("A", "B"); + WritePackage("B", "A"); + var rootDirectory = WorkDirectory.Combine("root").EnsureDirectoryExists(); + + var exception = Assert.Throws(() => Resolve(RootDependingOn("A"), rootDirectory)); + + // Naming only the package where the walk re-entered would leave the user hunting for the + // other half of the cycle. + Assert.That(exception!.Message, Does.Contain("A -> B -> A")); + } + + [Test] + public void SelfDependencyIsRejected() + { + WritePackage("Solo", "Solo"); + var rootDirectory = WorkDirectory.Combine("root").EnsureDirectoryExists(); + + var exception = Assert.Throws(() => Resolve(RootDependingOn("Solo"), rootDirectory)); + + Assert.That(exception!.Message, Does.Contain("depends on itself")); + } + + [Test] + public void ConflictingPinsAreAHardError() + { + // Two spellings of the same package name, pinned to different directories. + WorkDirectory.Combine("CopyOne").EnsureDirectoryExists(); + WorkDirectory.Combine("CopyTwo").EnsureDirectoryExists(); + var viaA = WorkDirectory.Combine("A").EnsureDirectoryExists(); + PackageManifest.PathIn(viaA).WriteAllText( + "{ \"dependencies\": { \"Shared\": { \"path\": \"../CopyTwo\" } } }"); + + var root = PackageManifest.Parse( + "{ \"dependencies\": { \"A\": { \"path\": \"../A\" }, " + + "\"Shared\": { \"path\": \"../CopyOne\" } } }", + WorkDirectory.Combine("root", PackageManifest.FileName)); + var rootDirectory = WorkDirectory.Combine("root").EnsureDirectoryExists(); + + var exception = Assert.Throws(() => Resolve(root, rootDirectory)); + + Assert.That(exception!.Message, Does.Contain("conflicting pins")); + Assert.That(exception.Message, Does.Contain("Shared")); + // The message has to say how to get unstuck, not just that something is wrong. + Assert.That(exception.Message, Does.Contain("overrides")); + } + + [Test] + public void AnOverrideResolvesAConflict() + { + WorkDirectory.Combine("CopyOne").EnsureDirectoryExists(); + WorkDirectory.Combine("CopyTwo").EnsureDirectoryExists(); + var viaA = WorkDirectory.Combine("A").EnsureDirectoryExists(); + PackageManifest.PathIn(viaA).WriteAllText( + "{ \"dependencies\": { \"Shared\": { \"path\": \"../CopyTwo\" } } }"); + + var root = PackageManifest.Parse( + "{ \"dependencies\": { \"A\": { \"path\": \"../A\" }, " + + "\"Shared\": { \"path\": \"../CopyOne\" } }, " + + "\"overrides\": { \"Shared\": { \"path\": \"" + WorkDirectory.Combine("CopyOne").ToString().Replace("\\", "\\\\") + "\" } } }", + WorkDirectory.Combine("root", PackageManifest.FileName)); + var rootDirectory = WorkDirectory.Combine("root").EnsureDirectoryExists(); + + var result = Resolve(root, rootDirectory); + + var shared = result.Packages.Single(package => package.Name == "Shared"); + Assert.That(shared.Root.FileName, Is.EqualTo("CopyOne")); + } + + [Test] + public void MissingPathDependencyNamesWhatItLookedFor() + { + var rootDirectory = WorkDirectory.Combine("root").EnsureDirectoryExists(); + + var exception = Assert.Throws( + () => Resolve(RootDependingOn("NotThere"), rootDirectory)); + + Assert.That(exception!.Message, Does.Contain("NotThere")); + } +} diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs b/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs new file mode 100644 index 0000000..09ecc16 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs @@ -0,0 +1,199 @@ +using System.Diagnostics; +using NiceIO; +using ReBuildTool.Service.PackageService; + +namespace ReBuildTool.Test; + +/// +/// End-to-end restore against a real git repository. +/// +/// The repository is created locally and cloned over a filesystem path, so the test exercises the +/// genuine clone / fetch / rev-parse / reset code path without ever touching the network - CI runs +/// this on three hosts and a flaky external dependency would be worse than no test at all. +/// +[TestFixture] +public class TestPackageRestore +{ + private NPath WorkDirectory = null!; + + [SetUp] + public void SetUp() + { + WorkDirectory = Path.Combine(Path.GetTempPath(), $"rbt-restore-{Guid.NewGuid():N}") + .ToNPath() + .EnsureDirectoryExists(); + } + + [TearDown] + public void TearDown() + { + // A clone contains read-only object files on Windows; ignore whatever will not go away. + try + { + WorkDirectory.DeleteIfExists(DeleteMode.Normal); + } + catch (Exception) + { + // Leaving a temp directory behind must never fail a test run. + } + } + + private static string Git(NPath workingDirectory, params string[] arguments) + { + var startInfo = new ProcessStartInfo + { + FileName = "git", + WorkingDirectory = workingDirectory.ToString(), + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true + }; + // A committing identity is configured per-command: CI runners have no global git identity. + startInfo.ArgumentList.Add("-c"); + startInfo.ArgumentList.Add("user.email=rbt@example.com"); + startInfo.ArgumentList.Add("-c"); + startInfo.ArgumentList.Add("user.name=rbt test"); + foreach (var argument in arguments) + { + startInfo.ArgumentList.Add(argument); + } + + using var process = Process.Start(startInfo)!; + var stdout = process.StandardOutput.ReadToEnd(); + var stderr = process.StandardError.ReadToEnd(); + process.WaitForExit(); + Assert.That(process.ExitCode, Is.EqualTo(0), + $"git {string.Join(" ", arguments)} failed: {stderr}"); + return stdout.Trim(); + } + + /// Builds a git repository holding one rbt module, tagged v1.0. + private NPath CreateLibraryRepository(string name) + { + var repository = WorkDirectory.Combine($"{name}Repo").EnsureDirectoryExists(); + repository.Combine($"{name}.module.cs").WriteAllText( + $"using ReBuildTool.ToolChain;{Environment.NewLine}" + + $"public class {name} : CppModuleRule {{ }}{Environment.NewLine}"); + PackageManifest.PathIn(repository).WriteAllText($"{{ \"name\": \"{name}\" }}"); + + Git(repository, "init", "--initial-branch=main"); + Git(repository, "add", "."); + Git(repository, "commit", "-m", "initial"); + Git(repository, "tag", "v1.0"); + return repository; + } + + private NPath CreateProject(string manifestJson) + { + var project = WorkDirectory.Combine("Project").EnsureDirectoryExists(); + PackageManifest.PathIn(project).WriteAllText(manifestJson); + return project; + } + + [Test] + public void AGitPackageIsClonedAndPinnedToItsTag() + { + var repository = CreateLibraryRepository("GreeterLib"); + var expectedSha = Git(repository, "rev-parse", "v1.0^{commit}"); + var project = CreateProject( + "{ \"dependencies\": { \"GreeterLib\": { " + + $"\"git\": \"{repository.ToString(SlashMode.Forward)}\", \"tag\": \"v1.0\" }} }} }}"); + + var result = new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + + Assert.That(result.Packages.Select(package => package.Name), Is.EqualTo(new[] { "GreeterLib" })); + Assert.That(project.Combine("Packages", "GreeterLib", "GreeterLib.module.cs").FileExists(), Is.True); + + // The lock records the commit the tag pointed at, not the tag: upstream can move a tag. + var lockFile = PackageLockFile.ReadFrom(project); + Assert.That(lockFile, Is.Not.Null); + Assert.That(lockFile!.Find("GreeterLib")!.Resolved, Is.EqualTo(expectedSha)); + } + + [Test] + public void ASecondRestoreSucceedsOffline() + { + var repository = CreateLibraryRepository("GreeterLib"); + var project = CreateProject( + "{ \"dependencies\": { \"GreeterLib\": { " + + $"\"git\": \"{repository.ToString(SlashMode.Forward)}\", \"tag\": \"v1.0\" }} }} }}"); + new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + + // Everything is already on disk and the lock pins a commit, so no remote access is needed. + Assert.DoesNotThrow(() => + new PackageRestoreService().Restore(project, new PackageRestoreOptions { Offline = true })); + } + + [Test] + public void AnUnfetchedPackageCannotBeRestoredOffline() + { + var repository = CreateLibraryRepository("GreeterLib"); + var project = CreateProject( + "{ \"dependencies\": { \"GreeterLib\": { " + + $"\"git\": \"{repository.ToString(SlashMode.Forward)}\", \"tag\": \"v1.0\" }} }} }}"); + + var exception = Assert.Throws(() => + new PackageRestoreService().Restore(project, new PackageRestoreOptions { Offline = true })); + + Assert.That(exception!.Message, Does.Contain("Offline")); + } + + [Test] + public void TransitiveGitDependenciesAreFollowed() + { + var leaf = CreateLibraryRepository("LeafLib"); + var middle = WorkDirectory.Combine("MiddleLibRepo").EnsureDirectoryExists(); + middle.Combine("MiddleLib.module.cs").WriteAllText( + $"using ReBuildTool.ToolChain;{Environment.NewLine}public class MiddleLib : CppModuleRule {{ }}"); + PackageManifest.PathIn(middle).WriteAllText( + "{ \"name\": \"MiddleLib\", \"dependencies\": { \"LeafLib\": { " + + $"\"git\": \"{leaf.ToString(SlashMode.Forward)}\", \"tag\": \"v1.0\" }} }} }}"); + Git(middle, "init", "--initial-branch=main"); + Git(middle, "add", "."); + Git(middle, "commit", "-m", "initial"); + Git(middle, "tag", "v1.0"); + + var project = CreateProject( + "{ \"dependencies\": { \"MiddleLib\": { " + + $"\"git\": \"{middle.ToString(SlashMode.Forward)}\", \"tag\": \"v1.0\" }} }} }}"); + + var result = new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + + // LeafLib is only discoverable by reading MiddleLib's manifest after it was cloned. + Assert.That( + result.Packages.Select(package => package.Name).OrderBy(name => name), + Is.EqualTo(new[] { "LeafLib", "MiddleLib" })); + Assert.That(project.Combine("Packages", "LeafLib").DirectoryExists(), Is.True); + } + + [Test] + public void AProjectWithoutAManifestIsUntouched() + { + var project = WorkDirectory.Combine("Bare").EnsureDirectoryExists(); + + var result = new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + + // Projects that do not use packages must not gain a Packages/ directory or a lock file. + Assert.That(result.Packages, Is.Empty); + Assert.That(project.Combine("Packages").DirectoryExists(), Is.False); + Assert.That(PackageLockFile.PathIn(project).FileExists(), Is.False); + } + + [Test] + public void RestoreAddsPackagesToGitIgnore() + { + var repository = CreateLibraryRepository("GreeterLib"); + var project = CreateProject( + "{ \"dependencies\": { \"GreeterLib\": { " + + $"\"git\": \"{repository.ToString(SlashMode.Forward)}\", \"tag\": \"v1.0\" }} }} }}"); + project.Combine(".gitignore").WriteAllText($"Intermedia{Environment.NewLine}"); + + new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + // A second restore must not append the pattern again. + new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + + var lines = project.Combine(".gitignore").ReadAllLines(); + Assert.That(lines.Count(line => line.Trim() == "/Packages/"), Is.EqualTo(1)); + } +} diff --git a/ReBuildTool/ReBuildTool/Program.cs b/ReBuildTool/ReBuildTool/Program.cs index e1cf9d1..9d21e13 100644 --- a/ReBuildTool/ReBuildTool/Program.cs +++ b/ReBuildTool/ReBuildTool/Program.cs @@ -59,6 +59,9 @@ case RunMode.ReBuild: project.ReBuild(targetName); break; + case RunMode.Restore: + // Parse() already restored; this mode just stops before doing anything else. + break; default: break; } diff --git a/Sample/GeometryPackage/GeometryModule.module.cs b/Sample/GeometryPackage/GeometryModule.module.cs new file mode 100644 index 0000000..0fe75cf --- /dev/null +++ b/Sample/GeometryPackage/GeometryModule.module.cs @@ -0,0 +1,19 @@ +using ReBuildTool.Service.CompileService; +using ReBuildTool.ToolChain; + +// A package ships ordinary module rules. Nothing here says "package": once restore has put the +// directory on disk, rbt globs this file into the very same CompileRules.dll as the consuming +// project's own rules, so the module behaves exactly like a local one. +// +// Note the module is named GeometryModule while the package is named GeometryPackage - a package +// is a unit of distribution and may contain any number of modules under any names. +public class GeometryModule : CppModuleRule +{ + public override void Setup(ICppBuildContext buildContext) + { + TargetBuildType = BuildType.StaticLibrary; + // Collapses the auto-generated GEOMETRYMODULE_API macro to nothing, matching how a static + // library is actually linked - same reasoning as Sample/StaticLibraryLink. + PublicDefines.Add("GEOMETRYMODULE_BUILT_AS_STATIC"); + } +} diff --git a/Sample/GeometryPackage/Private/GeometryModule.cpp b/Sample/GeometryPackage/Private/GeometryModule.cpp new file mode 100644 index 0000000..2f10e26 --- /dev/null +++ b/Sample/GeometryPackage/Private/GeometryModule.cpp @@ -0,0 +1,11 @@ +#include "GeometryModule.h" + +int GeometryRect::Area(int width, int height) +{ + return width * height; +} + +int GeometryRect::Perimeter(int width, int height) +{ + return 2 * (width + height); +} diff --git a/Sample/GeometryPackage/Public/GeometryModule.h b/Sample/GeometryPackage/Public/GeometryModule.h new file mode 100644 index 0000000..4c7a16a --- /dev/null +++ b/Sample/GeometryPackage/Public/GeometryModule.h @@ -0,0 +1,10 @@ +#pragma once + +#include "GeometryModule.internal.h" + +class GEOMETRYMODULE_API GeometryRect +{ +public: + int Area(int width, int height); + int Perimeter(int width, int height); +}; diff --git a/Sample/GeometryPackage/RBTPackage.json b/Sample/GeometryPackage/RBTPackage.json new file mode 100644 index 0000000..751b0a7 --- /dev/null +++ b/Sample/GeometryPackage/RBTPackage.json @@ -0,0 +1,3 @@ +{ + "name": "GeometryPackage" +} diff --git a/Sample/PackageConsumer/.gitignore b/Sample/PackageConsumer/.gitignore new file mode 100644 index 0000000..f5651e7 --- /dev/null +++ b/Sample/PackageConsumer/.gitignore @@ -0,0 +1,6 @@ +PackageConsumer.sln +BuildRule.sln +CompileRules.sln +Binary/ +Intermedia/ +/Packages/ diff --git a/Sample/PackageConsumer/RBTPackage.json b/Sample/PackageConsumer/RBTPackage.json new file mode 100644 index 0000000..fae5d71 --- /dev/null +++ b/Sample/PackageConsumer/RBTPackage.json @@ -0,0 +1,6 @@ +{ + "name": "PackageConsumer", + "dependencies": { + "GeometryPackage": { "path": "../GeometryPackage" } + } +} diff --git a/Sample/PackageConsumer/RBTPackage.lock.json b/Sample/PackageConsumer/RBTPackage.lock.json new file mode 100644 index 0000000..8164a45 --- /dev/null +++ b/Sample/PackageConsumer/RBTPackage.lock.json @@ -0,0 +1,13 @@ +{ + "version": 1, + "packages": [ + { + "name": "GeometryPackage", + "source": "Path", + "origin": "../GeometryPackage", + "resolved": "../GeometryPackage", + "pin": "path:../GeometryPackage", + "dependencies": [] + } + ] +} diff --git a/Sample/PackageConsumer/Source/AppModule/AppModule.module.cs b/Sample/PackageConsumer/Source/AppModule/AppModule.module.cs new file mode 100644 index 0000000..04d564a --- /dev/null +++ b/Sample/PackageConsumer/Source/AppModule/AppModule.module.cs @@ -0,0 +1,14 @@ +using ReBuildTool.Service.CompileService; +using ReBuildTool.ToolChain; + +public class AppModule : CppModuleRule +{ + public override void Setup(ICppBuildContext buildContext) + { + TargetBuildType = BuildType.Executable; + // GeometryModule is not in this project's Source/ - it comes from the GeometryPackage + // package declared in RBTPackage.json. A package's module is depended on by name, exactly + // like a local one. + Dependencies.Add("GeometryModule"); + } +} diff --git a/Sample/PackageConsumer/Source/AppModule/Private/AppModule.cpp b/Sample/PackageConsumer/Source/AppModule/Private/AppModule.cpp new file mode 100644 index 0000000..b13044b --- /dev/null +++ b/Sample/PackageConsumer/Source/AppModule/Private/AppModule.cpp @@ -0,0 +1,20 @@ +#include "AppModule.h" + +// Comes from the GeometryPackage package, not from this project's Source/. +#include "GeometryModule.h" + +#include + +void App::Run() +{ + GeometryRect rect; + printf("area(2, 3) = %d\n", rect.Area(2, 3)); + printf("perimeter(2, 3) = %d\n", rect.Perimeter(2, 3)); +} + +int main() +{ + App app; + app.Run(); + return 0; +} diff --git a/Sample/PackageConsumer/Source/AppModule/Public/AppModule.h b/Sample/PackageConsumer/Source/AppModule/Public/AppModule.h new file mode 100644 index 0000000..2209ca5 --- /dev/null +++ b/Sample/PackageConsumer/Source/AppModule/Public/AppModule.h @@ -0,0 +1,7 @@ +#pragma once + +class App +{ +public: + void Run(); +}; diff --git a/Sample/PackageConsumer/Source/PackageConsumerTarget.target.cs b/Sample/PackageConsumer/Source/PackageConsumerTarget.target.cs new file mode 100644 index 0000000..ab940a7 --- /dev/null +++ b/Sample/PackageConsumer/Source/PackageConsumerTarget.target.cs @@ -0,0 +1,9 @@ +using ReBuildTool.ToolChain; + +public class PackageConsumerTarget : CppTargetRule +{ + public PackageConsumerTarget() + { + UsedModules.Add("AppModule"); + } +} diff --git a/Sample/PackageConsumer/global.json b/Sample/PackageConsumer/global.json new file mode 100644 index 0000000..ecc6db8 --- /dev/null +++ b/Sample/PackageConsumer/global.json @@ -0,0 +1,7 @@ +{ + "sdk": { + "version": "8.0.0", + "rollForward": "latestMajor", + "allowPrerelease": true + } +} From b7130010edb91035284ab07d0f613c133b8c8a56 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 19:03:15 +0000 Subject: [PATCH 02/10] feat(package): HTTP archive source, binary packages and overlay rules Completes the three package shapes and adds the second remote source. HTTP archives (.zip/.tar.gz/.tgz/.tar) become a package source. This needed the three capabilities the repo had nowhere: Downloader, ArchiveExtractor and Hashing in ReBuildTool.Common/Misc. A URL is not self-verifying the way a commit is - the bytes behind it can change without the manifest changing - so a sha256 in the manifest is what makes an archive dependency reproducible, and a mismatch aborts the restore printing both hashes rather than unpacking whatever arrived. Extraction refuses any entry that resolves outside the destination (zip slip), supports strip-components because release tarballs wrap everything in one name-version/ directory, and unpacks through a staging directory so an interrupted run cannot leave a partial tree that a later restore accepts as complete. Prebuilt binary packages ship headers and libraries but no rule, so PackageModuleBinder generates one and PackageArtifactSelector picks the matching artifact by platform/arch/config - an omitted selector matches every value. Selection happens at Setup time rather than when the file is generated: baking the current platform in would change the file's content on every --TargetPlatform switch, and since the rule assembly is rebuilt off rule-file timestamps that would recompile every rule each time. Generated this way the file depends only on the manifest and never churns. Nothing matching is a warning, not a silent empty link. Generating a rule file rather than registering an IModuleInterface straight into ModuleRules keeps everything downstream working unchanged: the ModuleRulePaths lookup in InitAllRule, the SetupInternal lifecycle, the _API macro codegen, all four IDE generators and the HeaderTool plugin. Unmodified upstream source trees ship neither, so the consuming project supplies a rule through the dependency's "overlay". It is copied into the package root, not into a generated directory beside it, because an overlay's relative SourceDirectories/SourceFiles/Exclude* resolve against ModuleDirectory and only mean anything relative to the upstream tree. Tests: archive extraction including zip-slip refusal and strip-components, the download/checksum path served from a loopback HttpListener, artifact selection, and two integration tests that drive a whole project through restore, rule compilation and a real toolchain build - one consuming a binary package, one building upstream sources through an overlay. All offline. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019nEaEVhDjn2dQC19BE9iaM --- Doc/ARCH.md | 26 +- Doc/ARCH.zh-CN.md | 21 +- Doc/HowToUse.md | 63 ++++- Doc/HowToUse.zh-CN.md | 54 +++- .../Misc/ArchiveExtractor.cs | 151 +++++++++++ .../ReBuildTool.Common/Misc/Downloader.cs | 54 ++++ .../ReBuildTool.Common/Misc/Hashing.cs | 35 +++ .../Package/PackageArtifactSelector.cs | 94 +++++++ .../Package/PackageModuleBinder.cs | 120 ++++++++ .../Project/CppBuildProject.cs | 25 +- .../Fetchers/HttpArchivePackageFetcher.cs | 93 +++++++ .../PackageService/IPackageService.cs | 9 +- .../PackageService/PackageManifest.cs | 6 + .../PackageService/PackageResolver.cs | 36 ++- .../PackageService/PackageRestoreService.cs | 1 + .../ReBuildTool.Test/TestPackageArchive.cs | 256 ++++++++++++++++++ .../TestPackageBinaryModule.cs | 176 ++++++++++++ .../TestPackageBuildIntegration.cs | 167 ++++++++++++ 18 files changed, 1362 insertions(+), 25 deletions(-) create mode 100644 ReBuildTool/ReBuildTool.Common/Misc/ArchiveExtractor.cs create mode 100644 ReBuildTool/ReBuildTool.Common/Misc/Downloader.cs create mode 100644 ReBuildTool/ReBuildTool.Common/Misc/Hashing.cs create mode 100644 ReBuildTool/ReBuildTool.CppCompiler/Package/PackageArtifactSelector.cs create mode 100644 ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/HttpArchivePackageFetcher.cs create mode 100644 ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs create mode 100644 ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs create mode 100644 ReBuildTool/ReBuildTool.Test/TestPackageBuildIntegration.cs diff --git a/Doc/ARCH.md b/Doc/ARCH.md index f6ce168..957358e 100644 --- a/Doc/ARCH.md +++ b/Doc/ARCH.md @@ -142,14 +142,32 @@ Parse() │ ├─ PackageResolver: depth-first walk │ │ fetch → read the package's own manifest → recurse │ │ exact pins only; conflicting pins and cycles are hard errors - │ ├─ IPackageFetcher per source Git (clone/fetch/reset) | Path (used in place) + │ ├─ IPackageFetcher per source Git (clone/fetch/reset) + │ │ HttpArchive (download, sha256, unpack) + │ │ Path (used in place) │ └─ write RBTPackage.lock.json only when changed - └─ ParseRules() globs Source/ + each restored package root + ├─ PackageModuleBinder synthesizes a rule for a binary package; + │ installs a consumer-supplied overlay rule + └─ ParseRules() globs Source/ + package roots + generated rules ``` -Key types, all in `ReBuildTool.Service/PackageService/`: `PackageManifest`, +Key types in `ReBuildTool.Service/PackageService/`: `PackageManifest`, `PackageLockFile`, `PackageResolver`, `PackageRestoreService`, and -`Fetchers/IPackageFetcher`. The `--Offline` / `--ForceRestore` / `--UpdateLock` +`Fetchers/IPackageFetcher`. The download / extract / SHA256 helpers the archive +fetcher needs live in `ReBuildTool.Common/Misc/` (`Downloader`, +`ArchiveExtractor`, `Hashing`) — the first networking in rbt that is not a +shell-out to git. + +Above them, in `ReBuildTool.CppCompiler/Package/`: `PackageModuleBinder` +generates a module rule for a prebuilt binary package and installs a +consumer-supplied `overlay` rule, and `PackageArtifactSelector` picks the right +prebuilt artifact at `Setup` time. Generating a rule file, rather than +registering an `IModuleInterface` straight into `ModuleRules`, keeps everything +downstream working unchanged: the `ModuleRulePaths` lookup in `InitAllRule` +(which throws for a module it has no path for), the `SetupInternal` lifecycle, +the `_API` macro codegen, all four IDE generators and the HeaderTool plugin. + +The `--Offline` / `--ForceRestore` / `--UpdateLock` flags live in `ReBuildTool.CppCompiler/Project/PackageArgs.cs` — deliberately in that assembly rather than beside the service, because `CmdParser` discovers argument groups by scanning `AppDomain.CurrentDomain.GetAssemblies()` and .NET diff --git a/Doc/ARCH.zh-CN.md b/Doc/ARCH.zh-CN.md index 3d42e36..6de781d 100644 --- a/Doc/ARCH.zh-CN.md +++ b/Doc/ARCH.zh-CN.md @@ -119,14 +119,27 @@ Parse() │ ├─ PackageResolver：深度优先遍历 │ │ 拉取 → 读取该包自己的清单 → 递归 │ │ 只接受精确 pin；pin 冲突与依赖成环均为硬错误 - │ ├─ 按来源分派 IPackageFetcher Git（clone/fetch/reset）| Path（原地使用） + │ ├─ 按来源分派 IPackageFetcher Git（clone/fetch/reset） + │ │ HttpArchive（下载、sha256 校验、解压） + │ │ Path（原地使用） │ └─ 写出 RBTPackage.lock.json 仅在内容变化时 - └─ ParseRules() glob Source/ 以及每个已 restore 的包根目录 + ├─ PackageModuleBinder 为二进制包合成规则； + │ 安装消费方提供的 overlay 规则 + └─ ParseRules() glob Source/ + 包根目录 + 生成的规则目录 ``` -主要类型都在 `ReBuildTool.Service/PackageService/` 下：`PackageManifest`、 +主要类型在 `ReBuildTool.Service/PackageService/` 下：`PackageManifest`、 `PackageLockFile`、`PackageResolver`、`PackageRestoreService`，以及 -`Fetchers/IPackageFetcher`。`--Offline` / `--ForceRestore` / `--UpdateLock` 三个参数 +`Fetchers/IPackageFetcher`。压缩包 fetcher 需要的下载 / 解压 / SHA256 三个辅助类放在 +`ReBuildTool.Common/Misc/`（`Downloader`、`ArchiveExtractor`、`Hashing`）—— +这是 rbt 里第一处不靠 shell 调用 git 的网络访问。 + +在它们之上，`ReBuildTool.CppCompiler/Package/` 里：`PackageModuleBinder` 负责为预编译 +二进制包生成模块规则、并安装消费方提供的 `overlay` 规则；`PackageArtifactSelector` 负责在 +`Setup` 阶段挑选正确的预编译产物。选择「生成规则文件」而不是「直接把 `IModuleInterface` +注册进 `ModuleRules`」，是为了让下游的一切原样继续工作：`InitAllRule` 里的 +`ModuleRulePaths` 查找（找不到路径就抛异常）、`SetupInternal` 生命周期、`_API` 宏代码生成、 +四种 IDE 生成器，以及 HeaderTool 插件。`--Offline` / `--ForceRestore` / `--UpdateLock` 三个参数 定义在 `ReBuildTool.CppCompiler/Project/PackageArgs.cs` —— 特意放在该程序集而不是服务旁边， 因为 `CmdParser` 通过扫描 `AppDomain.CurrentDomain.GetAssemblies()` 发现参数组， 而 .NET 的程序集是惰性加载的。 diff --git a/Doc/HowToUse.md b/Doc/HowToUse.md index b184f01..89b012b 100644 --- a/Doc/HowToUse.md +++ b/Doc/HowToUse.md @@ -243,21 +243,30 @@ under `Packages/`, and records exactly what it resolved to in "GreeterLib": { "git": "https://github.com/x/greeter.git", "tag": "v1.2.0" }, // pinned to an exact commit "FooLib": { "git": "https://github.com/x/foo.git", "commit": "a1b2c3d4..." }, + // a release archive, verified against its hash + "zlib": { "url": "https://.../zlib-1.3.tar.gz", "sha256": "…", "strip": 1 }, // a directory on this machine, for local co-development "LocalLib": { "path": "../LocalLib" } } } ``` -Each dependency sets **exactly one** source (`git` or `path`), and a git source -must carry a `commit`, `tag` or `branch` — RBT resolves exact pins only and will -never pick a version for you. +Each dependency sets **exactly one** source (`git`, `url` or `path`), and a git +source must carry a `commit`, `tag` or `branch` — RBT resolves exact pins only +and will never pick a version for you. -### What a package is +`url` accepts `.zip`, `.tar.gz`/`.tgz` and `.tar`. `strip` drops that many leading +path components, like `tar --strip-components`, because release tarballs almost +always wrap everything in a single `name-version/` directory. A URL is not +self-verifying the way a commit is — the bytes behind it can change without the +manifest changing — so give it a `sha256`; a mismatch aborts the restore and +prints both hashes. -A package is just a directory containing `.module.cs` rule files. Once restored, -its rules are globbed into the very same `CompileRules.dll` as the project's own, -so a package module is depended on by name like any local one: +### The three shapes a package can take + +**1. A source package** ships its own `.module.cs`. Nothing special happens: its +rules are globbed into the very same `CompileRules.dll` as the project's own, so +its modules are depended on by name like any local one. ```csharp Dependencies.Add("GeometryModule"); @@ -269,6 +278,46 @@ name and its modules' names are independent; see [Sample/PackageConsumer](../Sample/PackageConsumer) and the package it consumes, [Sample/GeometryPackage](../Sample/GeometryPackage). +**2. A prebuilt binary package** ships headers and libraries but no rule. It +declares them in its own manifest and RBT synthesizes the module: + +```jsonc +{ + "name": "SomePrebuilt", + "binary": { + "module": "SomePrebuiltModule", // defaults to the package name + "includes": ["include"], + "artifacts": [ + { "platform": "Windows", "arch": "x64", "config": "Release", + "libraryDirectories": ["lib/win-x64"], "staticLibraries": ["some.lib"] }, + { "platform": "Linux", "arch": "x64", + "libraryDirectories": ["lib/linux-x64"], "staticLibraries": ["libsome.a"] } + ] + } +} +``` + +`platform` matches `--TargetPlatform`, `arch` matches `--TargetArch`, `config` +matches `--BuildConfig`; **omit one to match every value**. The right artifact is +picked while the build is being set up, not when the rule is generated, so +switching target platform does not invalidate anything. If nothing matches, RBT +warns rather than linking silently against nothing. + +**3. An unmodified upstream source tree** ships neither headers-and-libs nor a +rule — just somebody else's `src/` layout. The consuming project supplies the +rule with `overlay`: + +```jsonc +"glfw": { + "git": "https://github.com/glfw/glfw.git", "tag": "3.4", + "overlay": "Overlays/glfw.module.cs" +} +``` + +The overlay is copied into the package, so its relative `SourceDirectories`, +`SourceFiles`, `ExcludeDirectories` and `ExcludeFiles` resolve against the +upstream tree — which is exactly what those members exist for. + ### Transitive dependencies A package declares its own dependencies in its own `RBTPackage.json`, and RBT diff --git a/Doc/HowToUse.zh-CN.md b/Doc/HowToUse.zh-CN.md index 81341fc..bf5a56c 100644 --- a/Doc/HowToUse.zh-CN.md +++ b/Doc/HowToUse.zh-CN.md @@ -233,19 +233,26 @@ Target 规则不同：它的 `UsedModules` / `Plugins` 在任何 target `Setup` "GreeterLib": { "git": "https://github.com/x/greeter.git", "tag": "v1.2.0" }, // 固定到精确 commit "FooLib": { "git": "https://github.com/x/foo.git", "commit": "a1b2c3d4..." }, + // 发布压缩包，按哈希校验 + "zlib": { "url": "https://.../zlib-1.3.tar.gz", "sha256": "…", "strip": 1 }, // 本机上的目录，用于本地联调 "LocalLib": { "path": "../LocalLib" } } } ``` -每条依赖**有且只有一个**来源（`git` 或 `path`）；git 来源必须带上 `commit`、`tag` -或 `branch` —— RBT 只接受精确 pin，永远不会替你挑版本。 +每条依赖**有且只有一个**来源（`git`、`url` 或 `path`）；git 来源必须带上 `commit`、 +`tag` 或 `branch` —— RBT 只接受精确 pin，永远不会替你挑版本。 -### 什么是一个包 +`url` 支持 `.zip`、`.tar.gz`/`.tgz` 和 `.tar`。`strip` 会丢掉指定数量的前导路径段， +等同于 `tar --strip-components` —— 因为发布用的 tarball 基本都会把内容包在一层 +`name-version/` 目录里。URL 不像 commit 那样自带校验能力：它背后的字节可以在清单不变的 +情况下被换掉，所以请给它写上 `sha256`；不匹配会中止 restore 并打印两个哈希。 -包就是一个装着 `.module.cs` 规则文件的目录。restore 之后，它的规则会和项目自己的 -规则一起被 glob 进同一个 `CompileRules.dll`，因此包里的模块和本地模块一样按名字依赖： +### 包的三种形态 + +**1. 源码包**自带 `.module.cs`。没有任何特殊处理：它的规则会和项目自己的规则一起被 +glob 进同一个 `CompileRules.dll`，因此包里的模块和本地模块一样按名字依赖： ```csharp Dependencies.Add("GeometryModule"); @@ -256,6 +263,43 @@ Dependencies.Add("GeometryModule"); [Sample/PackageConsumer](../Sample/PackageConsumer) 及它消费的 [Sample/GeometryPackage](../Sample/GeometryPackage)。 +**2. 预编译二进制包**只带头文件和库，不带规则。它在自己的清单里声明产物，RBT 负责合成模块： + +```jsonc +{ + "name": "SomePrebuilt", + "binary": { + "module": "SomePrebuiltModule", // 缺省为包名 + "includes": ["include"], + "artifacts": [ + { "platform": "Windows", "arch": "x64", "config": "Release", + "libraryDirectories": ["lib/win-x64"], "staticLibraries": ["some.lib"] }, + { "platform": "Linux", "arch": "x64", + "libraryDirectories": ["lib/linux-x64"], "staticLibraries": ["libsome.a"] } + ] + } +} +``` + +`platform` 对应 `--TargetPlatform`，`arch` 对应 `--TargetArch`，`config` 对应 +`--BuildConfig`；**省略某一项即表示匹配全部取值**。产物是在构建 setup 阶段选择的， +不是在生成规则文件时选的，因此切换目标平台不会让任何缓存失效。若一条都匹配不上， +RBT 会给出警告，而不是悄悄链接一个空的。 + +**3. 第三方原样源码**既没有头文件+库，也没有规则 —— 就是别人的 `src/` 目录结构。 +由消费方项目通过 `overlay` 提供规则： + +```jsonc +"glfw": { + "git": "https://github.com/glfw/glfw.git", "tag": "3.4", + "overlay": "Overlays/glfw.module.cs" +} +``` + +overlay 会被复制进包内，这样它里面相对路径形式的 `SourceDirectories`、`SourceFiles`、 +`ExcludeDirectories`、`ExcludeFiles` 才能正确解析到上游代码树 —— 这几个成员本来就是 +为这类库准备的。 + ### 传递依赖 包在自己的 `RBTPackage.json` 里声明自己的依赖，RBT 会沿着图往下走：拉一个包，读它 diff --git a/ReBuildTool/ReBuildTool.Common/Misc/ArchiveExtractor.cs b/ReBuildTool/ReBuildTool.Common/Misc/ArchiveExtractor.cs new file mode 100644 index 0000000..6d4e748 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Common/Misc/ArchiveExtractor.cs @@ -0,0 +1,151 @@ +using System.Formats.Tar; +using System.IO.Compression; +using NiceIO; + +namespace ReBuildTool.Service.Global; + +/// +/// Unpacks the archive formats upstream projects publish releases as. Uses only what .NET 8 ships +/// (, , ) so no new dependency +/// enters the build. +/// +public static class ArchiveExtractor +{ + /// + /// Extracts into . + /// + /// drops that many leading path segments from every entry, + /// like tar --strip-components: release tarballs almost always wrap their contents in a + /// single name-version/ directory that nobody wants in the extracted tree. + /// + public static void Extract(NPath archive, NPath destination, int stripComponents = 0) + { + destination.EnsureDirectoryExists(); + var name = archive.FileName.ToLowerInvariant(); + + if (name.EndsWith(".zip")) + { + ExtractZip(archive, destination, stripComponents); + return; + } + if (name.EndsWith(".tar.gz") || name.EndsWith(".tgz")) + { + using var file = File.OpenRead(archive.ToString()); + using var gzip = new GZipStream(file, CompressionMode.Decompress); + ExtractTar(gzip, destination, stripComponents); + return; + } + if (name.EndsWith(".tar")) + { + using var file = File.OpenRead(archive.ToString()); + ExtractTar(file, destination, stripComponents); + return; + } + + throw new NotSupportedException( + $"cannot extract \"{archive.FileName}\": expected a .zip, .tar, .tar.gz or .tgz archive."); + } + + private static void ExtractZip(NPath archive, NPath destination, int stripComponents) + { + using var zip = ZipFile.OpenRead(archive.ToString()); + foreach (var entry in zip.Entries) + { + // A zip directory entry has an empty name and no content. + if (string.IsNullOrEmpty(entry.Name)) + { + continue; + } + var target = ResolveEntryPath(entry.FullName, destination, stripComponents); + if (target == null) + { + continue; + } + target.EnsureParentDirectoryExists(); + entry.ExtractToFile(target.ToString(), true); + } + } + + private static void ExtractTar(Stream stream, NPath destination, int stripComponents) + { + using var reader = new TarReader(stream); + while (reader.GetNextEntry() is { } entry) + { + var target = ResolveEntryPath(entry.Name, destination, stripComponents); + if (target == null) + { + continue; + } + + if (entry.EntryType is TarEntryType.Directory) + { + target.EnsureDirectoryExists(); + continue; + } + if (entry.EntryType is not (TarEntryType.RegularFile or TarEntryType.V7RegularFile)) + { + // Symlinks, devices and hard links are not something a source package needs, and + // extracting them safely is a different problem. Skip rather than half-support. + continue; + } + + target.EnsureParentDirectoryExists(); + entry.ExtractToFile(target.ToString(), true); + + // Packages ship helper scripts and prebuilt tools; losing the executable bit would + // make them unusable on Unix, and zip has no mode to preserve in the first place. + if (!OperatingSystem.IsWindows()) + { + // Owner read/write is forced on: an archive claiming mode 0 would otherwise + // produce a file rbt cannot read back, let alone delete. + File.SetUnixFileMode( + target.ToString(), + entry.Mode | UnixFileMode.UserRead | UnixFileMode.UserWrite); + } + } + } + + /// + /// Maps an archive entry name to a path inside , or null when the + /// entry is stripped away entirely. + /// + /// Rejects any entry that would escape the destination - the "zip slip" trap, where an archive + /// carries a name like ../../etc/cron.d/x and extraction quietly writes outside the tree + /// it was told to write into. + /// + private static NPath? ResolveEntryPath(string entryName, NPath destination, int stripComponents) + { + var segments = entryName + .Replace('\\', '/') + .Split('/', StringSplitOptions.RemoveEmptyEntries) + .ToList(); + + if (segments.Any(segment => segment == "..")) + { + throw new IOException( + $"refusing to extract \"{entryName}\": the archive entry escapes the destination directory."); + } + + if (stripComponents > 0) + { + if (segments.Count <= stripComponents) + { + return null; + } + segments = segments.Skip(stripComponents).ToList(); + } + + if (segments.Count == 0) + { + return null; + } + + var target = destination.Combine(string.Join("/", segments)); + if (!target.IsChildOf(destination)) + { + throw new IOException( + $"refusing to extract \"{entryName}\": it resolves outside the destination directory."); + } + return target; + } +} diff --git a/ReBuildTool/ReBuildTool.Common/Misc/Downloader.cs b/ReBuildTool/ReBuildTool.Common/Misc/Downloader.cs new file mode 100644 index 0000000..12d3cf2 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Common/Misc/Downloader.cs @@ -0,0 +1,54 @@ +using NiceIO; + +namespace ReBuildTool.Service.Global; + +/// +/// Minimal HTTP GET to a file. The first piece of networking in rbt that is not a shell-out to +/// git, so it deliberately stays small: one shared , redirects followed by +/// the handler, and no retry policy - a package fetch that fails should say so rather than stall a +/// build behind silent retries. +/// +public static class Downloader +{ + // One client for the process: a new HttpClient per call leaks sockets in TIME_WAIT. + private static readonly HttpClient Client = new() + { + Timeout = TimeSpan.FromMinutes(10) + }; + + /// + /// Downloads to , writing through a + /// temporary file so an interrupted transfer never leaves a truncated artifact that a later + /// run would mistake for a complete one. + /// + public static void Download(string url, NPath destination) + { + destination.EnsureParentDirectoryExists(); + var temporary = destination.Parent.Combine($"{destination.FileName}.partial"); + temporary.DeleteIfExists(); + + try + { + using (var response = Client.GetAsync(url, HttpCompletionOption.ResponseHeadersRead) + .GetAwaiter().GetResult()) + { + if (!response.IsSuccessStatusCode) + { + throw new IOException( + $"downloading {url} failed: HTTP {(int)response.StatusCode} {response.ReasonPhrase}"); + } + + using var source = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult(); + using var target = File.Create(temporary.ToString()); + source.CopyTo(target); + } + + destination.DeleteIfExists(); + temporary.Move(destination); + } + finally + { + temporary.DeleteIfExists(); + } + } +} diff --git a/ReBuildTool/ReBuildTool.Common/Misc/Hashing.cs b/ReBuildTool/ReBuildTool.Common/Misc/Hashing.cs new file mode 100644 index 0000000..9f17549 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Common/Misc/Hashing.cs @@ -0,0 +1,35 @@ +using System.Security.Cryptography; +using NiceIO; + +namespace ReBuildTool.Service.Global; + +/// +/// Content hashing for downloaded artifacts. +/// +/// rbt's incremental machinery is timestamp based everywhere else, which is right for build +/// outputs derived from local sources. It is not enough for bytes pulled off the network: a +/// download has to be checked against what the manifest said it should be, before it is trusted +/// enough to unpack. +/// +public static class Hashing +{ + public static string Sha256Of(NPath file) + { + using var stream = File.OpenRead(file.ToString()); + return Convert.ToHexString(SHA256.HashData(stream)).ToLowerInvariant(); + } + + /// + /// Compares a computed hash against an expected one, tolerating the common spellings users + /// paste in: mixed case, and an explicit "sha256:" prefix. + /// + public static bool Matches(string expected, string actual) + { + var normalized = expected.Trim(); + if (normalized.StartsWith("sha256:", StringComparison.OrdinalIgnoreCase)) + { + normalized = normalized.Substring("sha256:".Length); + } + return string.Equals(normalized, actual, StringComparison.OrdinalIgnoreCase); + } +} diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageArtifactSelector.cs b/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageArtifactSelector.cs new file mode 100644 index 0000000..96a9797 --- /dev/null +++ b/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageArtifactSelector.cs @@ -0,0 +1,94 @@ +using NiceIO; +using ReBuildTool.Service.PackageService; +using ResetCore.Common; + +namespace ReBuildTool.ToolChain.Package; + +/// +/// Fills a synthesized module rule from a binary package's artifact table, choosing the entry that +/// matches what is currently being built. +/// +/// The selection happens here, at Setup time, rather than when the rule file is generated. +/// Baking the current platform into the generated source would make its content change with every +/// --TargetPlatform / --BuildConfig switch, and since the rule assembly is rebuilt +/// whenever a rule file's timestamp moves, that would recompile every rule on each such switch. +/// Generated this way the file depends only on the manifest and never churns. +/// +public static class PackageArtifactSelector +{ + /// + /// Reads the package manifest at and applies the matching + /// artifacts to . Called from the generated rule's Setup. + /// + public static void Apply(CppModuleRule module, ICppBuildContext buildContext, string manifestPath) + { + var path = manifestPath.ToNPath(); + if (!path.FileExists()) + { + Log.Warning($"[package] {module.TargetName}: {path} is gone; the package was probably removed."); + return; + } + + var manifest = PackageManifest.Parse(path.ReadAllText(), path); + var binary = manifest.Binary; + if (binary == null) + { + return; + } + + var packageRoot = path.Parent; + foreach (var include in binary.Includes) + { + module.PublicIncludePaths.Add(Resolve(packageRoot, include)); + } + + var platform = IPlatformSupport.CurrentTargetPlatform.ToString(); + // Architecture is matched on CommandLineName - the spelling --TargetArch accepts - not on + // Name, which is the IDE display name ("ARM64" vs "arm64"). + var architecture = buildContext.CurrentBuildOption.Architecture.CommandLineName; + var configuration = buildContext.CurrentBuildOption.Configuration.ToString(); + + var matched = 0; + foreach (var artifact in binary.Artifacts) + { + if (!Matches(artifact.Platform, platform) + || !Matches(artifact.Arch, architecture) + || !Matches(artifact.Config, configuration)) + { + continue; + } + matched++; + + foreach (var directory in artifact.LibraryDirectories) + { + module.PublicLibraryDirectories.Add(Resolve(packageRoot, directory)); + } + // Library names are passed to the linker as-is: they may be plain names it resolves + // through the search paths above, so they must not be turned into paths. + module.PublicStaticLibraries.AddRange(artifact.StaticLibraries); + module.PublicDynamicLibraries.AddRange(artifact.DynamicLibraries); + module.PublicDefines.AddRange(artifact.Defines); + } + + if (matched == 0 && binary.Artifacts.Count > 0) + { + // Not fatal: a package may legitimately support only some platforms, and the consuming + // module can gate itself with IsSupport. But a silent empty link is far worse to debug. + Log.Warning( + $"[package] {module.TargetName} ships no prebuilt artifact for " + + $"{platform}/{architecture}/{configuration}; nothing will be linked from it."); + } + } + + /// A null or empty selector in the manifest means "every value". + private static bool Matches(string? declared, string actual) + { + return string.IsNullOrWhiteSpace(declared) + || string.Equals(declared, actual, StringComparison.OrdinalIgnoreCase); + } + + private static string Resolve(NPath packageRoot, string path) + { + return System.IO.Path.IsPathRooted(path) ? path : packageRoot.Combine(path).ToString(); + } +} diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs b/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs new file mode 100644 index 0000000..f6c02dc --- /dev/null +++ b/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs @@ -0,0 +1,120 @@ +using NiceIO; +using ReBuildTool.Service.PackageService; +using ResetCore.Common; + +namespace ReBuildTool.ToolChain.Package; + +/// +/// Turns restored packages into things the rule glob can pick up. +/// +/// Source packages need nothing - they already carry .module.cs files. The two shapes that +/// do need work are handled here: +/// +/// - a prebuilt binary package has headers and libraries but no rule, so a module rule is +/// generated for it; +/// - an unmodified upstream source tree has neither, so the consuming project supplies a +/// rule through the dependency's overlay field and it is copied into place. +/// +/// Generating a rule file, rather than registering an IModuleInterface directly into ModuleRules, +/// keeps everything downstream working for free: the ModuleRulePaths lookup in InitAllRule (which +/// throws for a module it has no path for), the SetupInternal lifecycle, the _API macro codegen, +/// all four IDE generators and the HeaderTool plugin. +/// +public static class PackageModuleBinder +{ + /// Generated rules live here, one directory per module, under Packages/. + public const string GeneratedFolderName = ".generated"; + + /// + /// Writes whatever rule files the restored packages imply and returns the extra directories + /// ParseRules should glob in addition to the packages themselves. + /// + public static List Bind(NPath packagesRoot, IEnumerable packages) + { + var roots = new List(); + foreach (var package in packages) + { + var generated = BindOne(packagesRoot, package); + if (generated != null) + { + roots.Add(generated); + } + } + return roots; + } + + private static NPath? BindOne(NPath packagesRoot, RestoredPackage package) + { + InstallOverlay(package); + + var binary = package.Manifest?.Binary; + if (binary == null) + { + return null; + } + + var moduleName = string.IsNullOrWhiteSpace(binary.Module) ? package.Name : binary.Module!; + var moduleDirectory = packagesRoot.Combine(GeneratedFolderName, moduleName); + // The framework unconditionally registers a module's Public/ and Private/ directories; the + // source globbing warns once per missing path, so create them rather than emit noise. + moduleDirectory.Combine("Public").EnsureDirectoryExists(); + moduleDirectory.Combine("Private").EnsureDirectoryExists(); + + var manifestPath = PackageManifest.PathIn(package.Root).ToString().Replace("\\", "\\\\"); + var content = + $"// Generated by rbt for the binary package \"{package.Name}\". Do not edit - it is{Environment.NewLine}" + + $"// rewritten from {PackageManifest.FileName} on every restore.{Environment.NewLine}" + + $"using ReBuildTool.Service.CompileService;{Environment.NewLine}" + + $"using ReBuildTool.ToolChain;{Environment.NewLine}" + + $"using ReBuildTool.ToolChain.Package;{Environment.NewLine}" + + $"{Environment.NewLine}" + + $"public class {moduleName} : CppModuleRule{Environment.NewLine}" + + $"{{{Environment.NewLine}" + + $" public override void Setup(ICppBuildContext buildContext){Environment.NewLine}" + + $" {{{Environment.NewLine}" + + $" TargetBuildType = BuildType.StaticLibrary;{Environment.NewLine}" + + // The artifact table is read at Setup time so this file stays platform-independent. + $" PackageArtifactSelector.Apply(this, buildContext, \"{manifestPath}\");{Environment.NewLine}" + + $" }}{Environment.NewLine}" + + $"}}{Environment.NewLine}"; + + WriteIfChanged(moduleDirectory.Combine($"{moduleName}.module.cs"), content); + Log.Info($"[package] {package.Name} provides prebuilt module {moduleName}"); + return moduleDirectory; + } + + /// + /// Copies a consumer-supplied rule file into the package it describes. + /// + /// It has to land in the package root, not in a generated directory alongside it: the rule's + /// ModuleDirectory becomes wherever the file sits, and every relative path an overlay + /// declares - SourceDirectories, SourceFiles, ExcludeDirectories - is + /// resolved against it. Those paths only make sense relative to the upstream tree. + /// + private static void InstallOverlay(RestoredPackage package) + { + if (package.Overlay == null) + { + return; + } + + var destination = package.Root.Combine(package.Overlay.FileName); + WriteIfChanged(destination, package.Overlay.ReadAllText()); + Log.Info($"[package] {package.Name} uses overlay rule {package.Overlay.FileName}"); + } + + /// + /// Only writes when the content differs. Rewriting unconditionally would bump the file's + /// timestamp on every restore, and NeedReBuildRuleAssembly compares timestamps - the whole rule + /// assembly would be recompiled on each build. Same reasoning as CppModuleRule.GenerateCode. + /// + internal static void WriteIfChanged(NPath path, string content) + { + if (path.FileExists() && path.ReadAllText() == content) + { + return; + } + path.EnsureParentDirectoryExists(); + path.WriteAllText(content); + } +} diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs b/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs index ef50038..a5ef0f5 100644 --- a/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs +++ b/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs @@ -8,6 +8,7 @@ using ReBuildTool.Service.IDEService.CMake; using ReBuildTool.Service.IDEService.VisualStudio; using ReBuildTool.Service.PackageService; +using ReBuildTool.ToolChain.Package; using ResetCore.Common; using ResetCore.Common.Parser.Ini; @@ -52,10 +53,10 @@ private void ParseRules() // Packages contribute modules, never targets: what to build is the consuming project's // decision, and a package's target would otherwise silently join the build. - foreach (var package in RestoredPackages) + foreach (var root in PackageRuleRoots) { - moduleFiles.AddRange(package.Root.Files($"*{ICppProject.ModuleDefineExtension}", true)); - extraFiles.AddRange(package.Root.Files($"*{ICppProject.ExtensionDefineExtension}", true)); + moduleFiles.AddRange(root.Files($"*{ICppProject.ModuleDefineExtension}", true)); + extraFiles.AddRange(root.Files($"*{ICppProject.ExtensionDefineExtension}", true)); } foreach (var targetFile in targetFiles) @@ -208,6 +209,18 @@ public void RestorePackages() var result = service.Value.Restore(ProjectRoot, PackageArgs.Get().ToRestoreOptions()); RestoredPackages.Clear(); RestoredPackages.AddRange(result.Packages); + + PackageRuleRoots.Clear(); + PackageRuleRoots.AddRange(RestoredPackages.Select(package => package.Root)); + if (RestoredPackages.Count > 0) + { + // Packages that ship prebuilt binaries (or upstream sources with no rule of their own) + // have their module rule synthesized here, into extra directories that get globbed + // alongside the packages themselves. + PackageRuleRoots.AddRange(PackageModuleBinder.Bind( + ProjectRoot.Combine(PackageRestoreService.PackagesFolderName), + RestoredPackages)); + } } public void Setup() @@ -582,6 +595,12 @@ private void PostCompile(CppBuilder builder) /// Packages materialized by the last , in dependency order. private List RestoredPackages { get; } = new(); + + /// + /// Directories globs for rule files on top of Source/: each + /// restored package, plus the generated-rule directories synthesized for binary packages. + /// + private List PackageRuleRoots { get; } = new(); private IAssemblyCompileUnit BuildRuleCompileUnit { get; set; } private NPath CppBuildRuleProjectOutput => IntermediaFolder.Combine("CppBuildRule/Project"); diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/HttpArchivePackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/HttpArchivePackageFetcher.cs new file mode 100644 index 0000000..82f9a70 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/HttpArchivePackageFetcher.cs @@ -0,0 +1,93 @@ +using NiceIO; +using ReBuildTool.Service.Global; +using ResetCore.Common; + +namespace ReBuildTool.Service.PackageService.Fetchers; + +/// +/// Downloads a release archive over HTTP and unpacks it into +/// <ProjectRoot>/Packages/<name>. +/// +/// Unlike a git pin, a URL is not self-verifying: the bytes behind it can change without the +/// manifest changing. A sha256 in the manifest is therefore how an archive dependency +/// becomes reproducible, and the extracted tree records the hash it was built from so a later +/// restore can tell "already correct" from "the pin moved". +/// +public class HttpArchivePackageFetcher : IPackageFetcher +{ + /// Records which archive the extracted directory came from. Kept beside the package, not inside it. + private const string StampFileName = ".rbt-archive-sha256"; + + public PackageSourceKind Kind => PackageSourceKind.HttpArchive; + + public FetchedPackage Fetch(FetchRequest request) + { + var url = request.Dependency.Url!; + var destination = request.DefaultDestination; + var stamp = request.PackagesRoot.Combine($"{request.Name}{StampFileName}"); + var expected = request.Dependency.Sha256; + + // Already unpacked from the very archive the manifest asks for: nothing to do, and no + // reason to touch the network. + if (!request.Options.Force && destination.DirectoryExists() && stamp.FileExists()) + { + var current = stamp.ReadAllText().Trim(); + if (expected == null || Hashing.Matches(expected, current)) + { + return new FetchedPackage(destination, current); + } + } + + if (request.Options.Offline) + { + throw new PackageException( + $"--Offline was requested but package \"{request.Name}\" still has to be downloaded " + + $"from {url}. Run a restore without --Offline first."); + } + + var download = request.PackagesRoot.Combine(".downloads", $"{request.Name}-{Path.GetFileName(new Uri(url).LocalPath)}"); + Log.Info($"[package] downloading {request.Name} from {url}"); + try + { + Downloader.Download(url, download); + } + catch (Exception e) when (e is not PackageException) + { + throw new PackageException($"package \"{request.Name}\": {e.Message}", e); + } + + var actual = Hashing.Sha256Of(download); + if (expected != null && !Hashing.Matches(expected, actual)) + { + download.DeleteIfExists(); + throw new PackageException( + $"package \"{request.Name}\" failed its checksum.{Environment.NewLine}" + + $" expected sha256: {expected}{Environment.NewLine}" + + $" actual sha256: {actual}{Environment.NewLine}" + + $"The bytes at {url} are not the ones this manifest was written against."); + } + + // Unpack into a scratch directory and swap it in, so an extraction that dies half way + // cannot leave a partial tree that the check above would later accept as complete. + var staging = request.PackagesRoot.Combine($".staging-{request.Name}"); + staging.DeleteIfExists(DeleteMode.Normal); + try + { + ArchiveExtractor.Extract(download, staging, request.Dependency.Strip); + destination.DeleteIfExists(DeleteMode.Normal); + staging.Move(destination); + } + catch (Exception e) when (e is not PackageException) + { + throw new PackageException($"package \"{request.Name}\": {e.Message}", e); + } + finally + { + staging.DeleteIfExists(DeleteMode.Normal); + download.DeleteIfExists(); + } + + stamp.WriteAllText(actual); + return new FetchedPackage(destination, actual); + } +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/IPackageService.cs b/ReBuildTool/ReBuildTool.Service/PackageService/IPackageService.cs index 8795165..843d2d7 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/IPackageService.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/IPackageService.cs @@ -20,11 +20,12 @@ public class PackageRestoreOptions /// public class RestoredPackage { - public RestoredPackage(string name, NPath root, PackageManifest? manifest) + public RestoredPackage(string name, NPath root, PackageManifest? manifest, NPath? overlay = null) { Name = name; Root = root; Manifest = manifest; + Overlay = overlay; } public string Name { get; } @@ -33,6 +34,12 @@ public RestoredPackage(string name, NPath root, PackageManifest? manifest) public NPath Root { get; } public PackageManifest? Manifest { get; } + + /// + /// A .module.cs supplied by the consuming project for a package that ships none of its + /// own - an unmodified upstream source tree. Already resolved to an absolute path. + /// + public NPath? Overlay { get; } } public class PackageRestoreResult diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs index 008f7e9..e91a8cc 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs @@ -129,6 +129,12 @@ public string PinKey(string packageName) /// public class PackageBinarySpec { + /// + /// Name of the module rbt synthesizes for these artifacts, and therefore the name consumers + /// put in Dependencies. Defaults to the package name. + /// + [JsonProperty("module")] public string? Module { get; set; } + [JsonProperty("includes")] public List Includes { get; set; } = new(); [JsonProperty("artifacts")] public List Artifacts { get; set; } = new(); diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs index af50e67..07f365d 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs @@ -1,4 +1,5 @@ using NiceIO; +using ReBuildTool.Service.CompileService; using ReBuildTool.Service.PackageService.Fetchers; using ResetCore.Common; @@ -134,7 +135,11 @@ private void ResolveOne(string name, PackageDependency declared, NPath declaring Resolved[name] = new ResolvedEntry { PinKey = pinKey, - Package = new RestoredPackage(name, fetched.Root, manifest), + Package = new RestoredPackage( + name, + fetched.Root, + manifest, + ResolveOverlay(name, dependency, declaringDirectory)), Locked = locked }; @@ -157,4 +162,33 @@ private void ResolveOne(string name, PackageDependency declared, NPath declaring Log.Info($"[package] {name} -> {Resolved[name].Locked.Resolved}"); } + + /// + /// Resolves a dependency's overlay against the manifest that declared it - the rule file + /// belongs to whoever is consuming the package, not to the package itself. + /// + private static NPath? ResolveOverlay(string name, PackageDependency dependency, NPath declaringDirectory) + { + if (string.IsNullOrWhiteSpace(dependency.Overlay)) + { + return null; + } + + var overlay = System.IO.Path.IsPathRooted(dependency.Overlay) + ? dependency.Overlay.ToNPath() + : declaringDirectory.Combine(dependency.Overlay); + if (!overlay.FileExists()) + { + throw new PackageException( + $"package \"{name}\" declares overlay \"{dependency.Overlay}\", which resolves to " + + $"\"{overlay}\" - that file does not exist."); + } + if (!overlay.FileName.EndsWith(ICppProject.ModuleDefineExtension, StringComparison.OrdinalIgnoreCase)) + { + throw new PackageException( + $"package \"{name}\": overlay \"{dependency.Overlay}\" must be a " + + $"{ICppProject.ModuleDefineExtension} file."); + } + return overlay; + } } diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs index 69cb040..9905bbe 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs @@ -49,6 +49,7 @@ private static IEnumerable CreateFetchers() { yield return new GitPackageFetcher(); yield return new PathPackageFetcher(); + yield return new HttpArchivePackageFetcher(); } /// diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs b/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs new file mode 100644 index 0000000..4ada9bc --- /dev/null +++ b/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs @@ -0,0 +1,256 @@ +using System.Formats.Tar; +using System.IO.Compression; +using System.Net; +using NiceIO; +using ReBuildTool.Service.Global; +using ReBuildTool.Service.PackageService; + +namespace ReBuildTool.Test; + +/// +/// Archive extraction and the HTTP package source. +/// +/// The archives are built by the test and served from a loopback , so +/// the real download / checksum / unpack path runs end to end without depending on any external +/// host being up. +/// +[TestFixture] +public class TestPackageArchive +{ + private NPath WorkDirectory = null!; + + [SetUp] + public void SetUp() + { + WorkDirectory = Path.Combine(Path.GetTempPath(), $"rbt-archive-{Guid.NewGuid():N}") + .ToNPath() + .EnsureDirectoryExists(); + } + + [TearDown] + public void TearDown() + { + try + { + WorkDirectory.DeleteIfExists(DeleteMode.Normal); + } + catch (Exception) + { + // A leftover temp directory must never fail a test run. + } + } + + /// Builds a zip whose entries are all under a single top-level directory. + private NPath CreateZip(string archiveName, string topLevel, params (string Path, string Content)[] entries) + { + var archive = WorkDirectory.Combine(archiveName); + using var stream = File.Create(archive.ToString()); + using var zip = new ZipArchive(stream, ZipArchiveMode.Create); + foreach (var (path, content) in entries) + { + var entry = zip.CreateEntry($"{topLevel}/{path}"); + using var writer = new StreamWriter(entry.Open()); + writer.Write(content); + } + return archive; + } + + private NPath CreateTarGz(string archiveName, string topLevel, params (string Path, string Content)[] entries) + { + var staging = WorkDirectory.Combine($"staging-{Guid.NewGuid():N}").EnsureDirectoryExists(); + foreach (var (path, content) in entries) + { + var file = staging.Combine(topLevel, path); + file.EnsureParentDirectoryExists(); + file.WriteAllText(content); + } + + var tar = WorkDirectory.Combine($"{archiveName}.tar"); + TarFile.CreateFromDirectory(staging.ToString(), tar.ToString(), false); + + var archive = WorkDirectory.Combine(archiveName); + using (var input = File.OpenRead(tar.ToString())) + using (var output = File.Create(archive.ToString())) + using (var gzip = new GZipStream(output, CompressionMode.Compress)) + { + input.CopyTo(gzip); + } + tar.DeleteIfExists(); + staging.DeleteIfExists(DeleteMode.Normal); + return archive; + } + + /// Serves a single file on loopback for the lifetime of the returned disposable. + private sealed class LocalServer : IDisposable + { + private readonly HttpListener Listener; + + public LocalServer(NPath file) + { + // Port 0 is not available through HttpListener, so probe upward for a free one. + for (var port = 18800; port < 18900; port++) + { + var listener = new HttpListener(); + listener.Prefixes.Add($"http://127.0.0.1:{port}/"); + try + { + listener.Start(); + Listener = listener; + Url = $"http://127.0.0.1:{port}/{file.FileName}"; + break; + } + catch (HttpListenerException) + { + listener.Close(); + } + } + if (Listener == null) + { + throw new InvalidOperationException("no free loopback port for the test server"); + } + + var bytes = File.ReadAllBytes(file.ToString()); + Task.Run(() => + { + while (Listener.IsListening) + { + HttpListenerContext context; + try + { + context = Listener.GetContext(); + } + catch (Exception) + { + return; + } + context.Response.ContentLength64 = bytes.Length; + context.Response.OutputStream.Write(bytes, 0, bytes.Length); + context.Response.OutputStream.Close(); + } + }); + } + + public string Url { get; } = string.Empty; + + public void Dispose() + { + Listener.Stop(); + Listener.Close(); + } + } + + private NPath CreateProject(string manifestJson) + { + var project = WorkDirectory.Combine("Project").EnsureDirectoryExists(); + PackageManifest.PathIn(project).WriteAllText(manifestJson); + return project; + } + + [Test] + public void StripComponentsDropsTheWrapperDirectory() + { + var archive = CreateZip("lib.zip", "libfoo-1.2.3", ("include/foo.h", "#pragma once")); + var destination = WorkDirectory.Combine("out"); + + ArchiveExtractor.Extract(archive, destination, 1); + + // Without the strip the header would land under out/libfoo-1.2.3/include, which is not + // what any manifest wants to write include paths against. + Assert.That(destination.Combine("include", "foo.h").FileExists(), Is.True); + Assert.That(destination.Combine("libfoo-1.2.3").DirectoryExists(), Is.False); + } + + [Test] + public void TarGzIsExtracted() + { + var archive = CreateTarGz("lib.tar.gz", "libfoo-1.2.3", ("include/foo.h", "#pragma once")); + var destination = WorkDirectory.Combine("out"); + + ArchiveExtractor.Extract(archive, destination, 1); + + Assert.That(destination.Combine("include", "foo.h").FileExists(), Is.True); + } + + /// + /// The "zip slip" trap: an archive entry whose name climbs out of the destination. Extracting + /// it would write wherever the attacker named, so it has to be refused outright. + /// + [Test] + public void AnEntryEscapingTheDestinationIsRefused() + { + var archive = WorkDirectory.Combine("evil.zip"); + using (var stream = File.Create(archive.ToString())) + using (var zip = new ZipArchive(stream, ZipArchiveMode.Create)) + { + var entry = zip.CreateEntry("../escaped.txt"); + using var writer = new StreamWriter(entry.Open()); + writer.Write("pwned"); + } + + var destination = WorkDirectory.Combine("out"); + Assert.Throws(() => ArchiveExtractor.Extract(archive, destination, 0)); + Assert.That(WorkDirectory.Combine("escaped.txt").FileExists(), Is.False); + } + + [Test] + public void AnArchivePackageIsDownloadedAndUnpacked() + { + var archive = CreateZip("geo.zip", "geo-1.0", + ("Geo.module.cs", "using ReBuildTool.ToolChain; public class Geo : CppModuleRule { }"), + (PackageManifest.FileName, "{ \"name\": \"Geo\" }")); + var sha = Hashing.Sha256Of(archive); + using var server = new LocalServer(archive); + var project = CreateProject( + "{ \"dependencies\": { \"Geo\": { " + + $"\"url\": \"{server.Url}\", \"sha256\": \"{sha}\", \"strip\": 1 }} }} }}"); + + var result = new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + + Assert.That(result.Packages.Select(package => package.Name), Is.EqualTo(new[] { "Geo" })); + Assert.That(project.Combine("Packages", "Geo", "Geo.module.cs").FileExists(), Is.True); + Assert.That(PackageLockFile.ReadFrom(project)!.Find("Geo")!.Resolved, Is.EqualTo(sha)); + } + + [Test] + public void AChecksumMismatchIsFatalAndExplainsItself() + { + var archive = CreateZip("geo.zip", "geo-1.0", ("Geo.module.cs", "// content")); + using var server = new LocalServer(archive); + var wrong = new string('a', 64); + var project = CreateProject( + "{ \"dependencies\": { \"Geo\": { " + + $"\"url\": \"{server.Url}\", \"sha256\": \"{wrong}\", \"strip\": 1 }} }} }}"); + + var exception = Assert.Throws( + () => new PackageRestoreService().Restore(project, new PackageRestoreOptions())); + + // The message has to show both hashes; "checksum failed" alone tells the user nothing. + Assert.That(exception!.Message, Does.Contain(wrong)); + Assert.That(exception.Message, Does.Contain(Hashing.Sha256Of(archive))); + Assert.That(project.Combine("Packages", "Geo").DirectoryExists(), Is.False); + } + + [Test] + public void ASecondRestoreOfAnArchivePackageNeedsNoNetwork() + { + var archive = CreateZip("geo.zip", "geo-1.0", (PackageManifest.FileName, "{ \"name\": \"Geo\" }")); + var sha = Hashing.Sha256Of(archive); + var manifest = + "{ \"dependencies\": { \"Geo\": { " + + $"\"url\": \"http://127.0.0.1:18999/geo.zip\", \"sha256\": \"{sha}\", \"strip\": 1 }} }} }}"; + + NPath project; + using (var server = new LocalServer(archive)) + { + project = CreateProject( + "{ \"dependencies\": { \"Geo\": { " + + $"\"url\": \"{server.Url}\", \"sha256\": \"{sha}\", \"strip\": 1 }} }} }}"); + new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + } + + // The server is gone; the already-unpacked tree must be recognised by its recorded hash. + PackageManifest.PathIn(project).WriteAllText(manifest); + Assert.DoesNotThrow(() => + new PackageRestoreService().Restore(project, new PackageRestoreOptions { Offline = true })); + } +} diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs b/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs new file mode 100644 index 0000000..7e33af7 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs @@ -0,0 +1,176 @@ +using NiceIO; +using ReBuildTool.Service.PackageService; +using ReBuildTool.ToolChain; +using ReBuildTool.ToolChain.Package; +using ResetCore.Common; + +namespace ReBuildTool.Test; + +/// +/// The two package shapes that carry no rbt rule of their own: a prebuilt binary package, for which +/// rbt synthesizes a module rule, and an unmodified upstream source tree, for which the consuming +/// project supplies one through overlay. +/// +[TestFixture] +public class TestPackageBinaryModule +{ + private NPath WorkDirectory = null!; + + [SetUp] + public void SetUp() + { + WorkDirectory = Path.Combine(Path.GetTempPath(), $"rbt-binary-{Guid.NewGuid():N}") + .ToNPath() + .EnsureDirectoryExists(); + } + + [TearDown] + public void TearDown() + { + try + { + WorkDirectory.DeleteIfExists(DeleteMode.Normal); + } + catch (Exception) + { + // A leftover temp directory must never fail a test run. + } + } + + private RestoredPackage BinaryPackage(string name, string binaryJson, NPath? overlay = null) + { + var root = WorkDirectory.Combine(name).EnsureDirectoryExists(); + var json = $"{{ \"name\": \"{name}\", \"binary\": {binaryJson} }}"; + PackageManifest.PathIn(root).WriteAllText(json); + return new RestoredPackage(name, root, PackageManifest.Parse(json, root), overlay); + } + + [Test] + public void ABinaryPackageGetsAGeneratedModuleRule() + { + var package = BinaryPackage("PrebuiltPack", + "{ \"module\": \"PrebuiltModule\", \"includes\": [\"include\"], \"artifacts\": [] }"); + var packagesRoot = WorkDirectory.Combine("Packages"); + + var roots = PackageModuleBinder.Bind(packagesRoot, new[] { package }); + + var generated = packagesRoot.Combine( + PackageModuleBinder.GeneratedFolderName, "PrebuiltModule", "PrebuiltModule.module.cs"); + Assert.That(generated.FileExists(), Is.True); + Assert.That(roots.Any(root => root == generated.Parent), Is.True, + "the generated rule directory must be handed back as a glob root"); + Assert.That(generated.ReadAllText(), Does.Contain("class PrebuiltModule : CppModuleRule")); + // The framework registers a module's Public/Private dirs unconditionally; they must exist + // or every build logs a missing-path warning for them. + Assert.That(generated.Parent.Combine("Public").DirectoryExists(), Is.True); + Assert.That(generated.Parent.Combine("Private").DirectoryExists(), Is.True); + } + + [Test] + public void TheModuleNameDefaultsToThePackageName() + { + var package = BinaryPackage("SoloPack", "{ \"artifacts\": [] }"); + var packagesRoot = WorkDirectory.Combine("Packages"); + + PackageModuleBinder.Bind(packagesRoot, new[] { package }); + + Assert.That( + packagesRoot.Combine(PackageModuleBinder.GeneratedFolderName, "SoloPack", "SoloPack.module.cs") + .FileExists(), + Is.True); + } + + /// + /// The generated file must not encode the platform being built: NeedReBuildRuleAssembly + /// compares timestamps, so a file that changed content per --TargetPlatform would recompile + /// every rule on each switch. + /// + [Test] + public void TheGeneratedRuleIsPlatformIndependentAndDoesNotChurn() + { + var package = BinaryPackage("PrebuiltPack", + "{ \"artifacts\": [ { \"platform\": \"Windows\", \"arch\": \"x64\", " + + "\"staticLibraries\": [\"some.lib\"] } ] }"); + var packagesRoot = WorkDirectory.Combine("Packages"); + PackageModuleBinder.Bind(packagesRoot, new[] { package }); + var generated = packagesRoot.Combine( + PackageModuleBinder.GeneratedFolderName, "PrebuiltPack", "PrebuiltPack.module.cs"); + var writtenAt = File.GetLastWriteTimeUtc(generated); + Assert.That(generated.ReadAllText(), Does.Not.Contain("some.lib")); + + Thread.Sleep(1100); + PackageModuleBinder.Bind(packagesRoot, new[] { package }); + + Assert.That(File.GetLastWriteTimeUtc(generated), Is.EqualTo(writtenAt)); + } + + [Test] + public void AnOverlayIsCopiedIntoThePackageItDescribes() + { + var overlay = WorkDirectory.Combine("Overlays", "upstream.module.cs"); + overlay.EnsureParentDirectoryExists(); + overlay.WriteAllText("public class upstream { }"); + var root = WorkDirectory.Combine("UpstreamPack").EnsureDirectoryExists(); + var package = new RestoredPackage("UpstreamPack", root, null, overlay); + + PackageModuleBinder.Bind(WorkDirectory.Combine("Packages"), new[] { package }); + + // Into the package root, so the overlay's relative SourceDirectories/ExcludeFiles resolve + // against the upstream tree rather than against some generated directory. + Assert.That(root.Combine("upstream.module.cs").FileExists(), Is.True); + } + + private static ICppBuildContext BuildContext() + { + CmdParser.Parse(); + return new CppBuilder(); + } + + [Test] + public void ArtifactsAreSelectedByPlatformArchAndConfig() + { + var context = BuildContext(); + var platform = IPlatformSupport.CurrentTargetPlatform.ToString(); + var arch = context.CurrentBuildOption.Architecture.CommandLineName; + var config = context.CurrentBuildOption.Configuration.ToString(); + + var root = WorkDirectory.Combine("Pack").EnsureDirectoryExists(); + PackageManifest.PathIn(root).WriteAllText( + "{ \"name\": \"Pack\", \"binary\": { \"includes\": [\"include\"], \"artifacts\": [" + + $"{{ \"platform\": \"{platform}\", \"arch\": \"{arch}\", \"config\": \"{config}\", " + + "\"libraryDirectories\": [\"lib\"], \"staticLibraries\": [\"wanted\"] }, " + + "{ \"platform\": \"NotAPlatform\", \"staticLibraries\": [\"unwanted\"] } ] } }"); + + var module = new SyntheticModule { ModuleDirectoryForTest = root.ToString() }; + PackageArtifactSelector.Apply(module, context, PackageManifest.PathIn(root).ToString()); + + Assert.That(module.PublicStaticLibraries, Is.EqualTo(new[] { "wanted" })); + // Include and library directories are package-relative and must come back absolute. + Assert.That(module.PublicIncludePaths.Single(), Is.EqualTo(root.Combine("include").ToString())); + Assert.That(module.PublicLibraryDirectories.Single(), Is.EqualTo(root.Combine("lib").ToString())); + } + + [Test] + public void AnArtifactWithoutSelectorsMatchesEveryPlatform() + { + var context = BuildContext(); + var root = WorkDirectory.Combine("Pack").EnsureDirectoryExists(); + PackageManifest.PathIn(root).WriteAllText( + "{ \"name\": \"Pack\", \"binary\": { \"artifacts\": [ " + + "{ \"staticLibraries\": [\"everywhere\"] } ] } }"); + + var module = new SyntheticModule { ModuleDirectoryForTest = root.ToString() }; + PackageArtifactSelector.Apply(module, context, PackageManifest.PathIn(root).ToString()); + + Assert.That(module.PublicStaticLibraries, Is.EqualTo(new[] { "everywhere" })); + } + + /// Stands in for the rule rbt generates, so the selector can be tested on its own. + private class SyntheticModule : CppModuleRule + { + public string ModuleDirectoryForTest + { + set => ModuleDirectory = value; + } + } +} diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageBuildIntegration.cs b/ReBuildTool/ReBuildTool.Test/TestPackageBuildIntegration.cs new file mode 100644 index 0000000..9834797 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Test/TestPackageBuildIntegration.cs @@ -0,0 +1,167 @@ +using NiceIO; +using ReBuildTool.Service.CompileService; +using ReBuildTool.Service.Context; +using ReBuildTool.Service.PackageService; +using ReBuildTool.ToolChain.Package; +using ResetCore.Common; + +namespace ReBuildTool.Test; + +/// +/// Drives a whole project through restore, rule compilation and a real toolchain build, to prove a +/// package's module actually reaches the compiler rather than merely being resolved. +/// +/// Everything is generated into a temp directory and uses path dependencies, so it stays offline +/// and deterministic on all three CI hosts. +/// +[TestFixture] +public class TestPackageBuildIntegration +{ + private NPath WorkDirectory = null!; + + [SetUp] + public void SetUp() + { + WorkDirectory = Path.Combine(Path.GetTempPath(), $"rbt-integration-{Guid.NewGuid():N}") + .ToNPath() + .EnsureDirectoryExists(); + } + + [TearDown] + public void TearDown() + { + try + { + WorkDirectory.DeleteIfExists(DeleteMode.Normal); + } + catch (Exception) + { + // A leftover temp directory must never fail a test run. + } + } + + private NPath CreateConsumingProject(string manifestJson, string dependencyModule) + { + var project = WorkDirectory.Combine("Consumer").EnsureDirectoryExists(); + PackageManifest.PathIn(project).WriteAllText(manifestJson); + + var source = project.Combine("Source").EnsureDirectoryExists(); + source.Combine("ConsumerTarget.target.cs").WriteAllText( + "using ReBuildTool.ToolChain;\n" + + "public class ConsumerTarget : CppTargetRule\n" + + "{\n" + + " public ConsumerTarget() { UsedModules.Add(\"ConsumerModule\"); }\n" + + "}\n"); + + var module = source.Combine("ConsumerModule").EnsureDirectoryExists(); + module.Combine("ConsumerModule.module.cs").WriteAllText( + "using ReBuildTool.Service.CompileService;\n" + + "using ReBuildTool.ToolChain;\n" + + "public class ConsumerModule : CppModuleRule\n" + + "{\n" + + " public override void Setup(ICppBuildContext buildContext)\n" + + " {\n" + + " TargetBuildType = BuildType.Executable;\n" + + $" Dependencies.Add(\"{dependencyModule}\");\n" + + " }\n" + + "}\n"); + module.Combine("Public").EnsureDirectoryExists().Combine("ConsumerModule.h").WriteAllText( + "#pragma once\n"); + module.Combine("Private").EnsureDirectoryExists().Combine("ConsumerModule.cpp").WriteAllText( + "#include \"vendorlib.h\"\n" + + "#include \n" + + "int main()\n" + + "{\n" + + " printf(\"%d\\n\", vendor_answer());\n" + + " return vendor_answer() == 42 ? 0 : 1;\n" + + "}\n"); + return project; + } + + private static void Build(NPath project) + { + CmdParser.Parse(); + ServiceContext.Instance.Init(); + var cppProject = ServiceContext.Instance.Create(project).Value; + cppProject.Parse(); + cppProject.Setup(); + cppProject.Build(); + } + + /// + /// A header-only prebuilt package: no rule of its own, so rbt synthesizes one, and the include + /// path it declares has to reach the consuming module's compile line for this to link. + /// + [Test] + public void ABinaryPackageIsGeneratedIntoTheBuildAndCompiles() + { + var package = WorkDirectory.Combine("VendorPack").EnsureDirectoryExists(); + PackageManifest.PathIn(package).WriteAllText( + "{ \"name\": \"VendorPack\", \"binary\": { \"module\": \"VendorModule\", " + + "\"includes\": [\"include\"], \"defines\": [], \"artifacts\": [] } }"); + var include = package.Combine("include").EnsureDirectoryExists(); + include.Combine("vendorlib.h").WriteAllText( + "#pragma once\ninline int vendor_answer() { return 42; }\n"); + + var project = CreateConsumingProject( + "{ \"dependencies\": { \"VendorPack\": { \"path\": \"../VendorPack\" } } }", + "VendorModule"); + + Build(project); + + // The rule rbt generated for the package has to have been compiled into the rule assembly + // and produced a real library, and the executable must have linked against it. + var generated = project.Combine( + "Packages", PackageModuleBinder.GeneratedFolderName, "VendorModule", "VendorModule.module.cs"); + Assert.That(generated.FileExists(), Is.True, "the binary package's module rule should be generated"); + + var binaries = project.Combine("Binary").Files(true).Select(file => file.FileName).ToList(); + Assert.That(binaries.Any(name => name.StartsWith("ConsumerModule")), Is.True, + $"the executable should have been produced, got: {string.Join(", ", binaries)}"); + } + + /// + /// An unmodified upstream tree: it ships sources but no rbt rule, and the consuming project + /// supplies one through overlay. + /// + [Test] + public void AnOverlayRuleBuildsUpstreamSources() + { + var upstream = WorkDirectory.Combine("Upstream").EnsureDirectoryExists(); + upstream.Combine("include").EnsureDirectoryExists().Combine("vendorlib.h").WriteAllText( + "#pragma once\nint vendor_answer();\n"); + upstream.Combine("src").EnsureDirectoryExists().Combine("vendorlib.cpp").WriteAllText( + "#include \"vendorlib.h\"\nint vendor_answer() { return 42; }\n"); + + var project = CreateConsumingProject( + "{ \"dependencies\": { \"Upstream\": { \"path\": \"../Upstream\", " + + "\"overlay\": \"Overlays/VendorModule.module.cs\" } } }", + "VendorModule"); + + // The overlay describes how to build somebody else's source layout - exactly what + // SourceDirectories and the include paths exist for. + var overlay = project.Combine("Overlays", "VendorModule.module.cs"); + overlay.EnsureParentDirectoryExists(); + overlay.WriteAllText( + "using ReBuildTool.Service.CompileService;\n" + + "using ReBuildTool.ToolChain;\n" + + "public class VendorModule : CppModuleRule\n" + + "{\n" + + " public override void Setup(ICppBuildContext buildContext)\n" + + " {\n" + + " TargetBuildType = BuildType.StaticLibrary;\n" + + " PublicDefines.Add(\"VENDORMODULE_BUILT_AS_STATIC\");\n" + + " PublicIncludePaths.Add(\"include\");\n" + + " SourceDirectories.Add(\"src\");\n" + + " }\n" + + "}\n"); + + Build(project); + + // The overlay must land in the package root, or its relative "src"/"include" would not resolve. + Assert.That(upstream.Combine("VendorModule.module.cs").FileExists(), Is.True); + var binaries = project.Combine("Binary").Files(true).Select(file => file.FileName).ToList(); + Assert.That(binaries.Any(name => name.StartsWith("ConsumerModule")), Is.True, + $"the executable should have been produced, got: {string.Join(", ", binaries)}"); + } +} From 01058587b7a2a73aea9dfcee0cf3e0a912d43f50 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 19:09:52 +0000 Subject: [PATCH 03/10] feat(package): vcpkg bridge and --PackageAdd/--PackageRemove Completes the four package sources and adds command-line manifest editing. The vcpkg fetcher lets vcpkg do the acquiring and building, then translates: it writes an RBTPackage.json describing the installed tree as a prebuilt binary package, so PackageModuleBinder and PackageArtifactSelector take it from there and nothing downstream needs to know vcpkg exists. vcpkg's parallel debug/ prefix is mapped onto rbt's Debug configuration, with an explicit artifact per release configuration - an artifact with no config would also match Debug and both library sets would end up on the link line. A port that built no debug variant has its release libraries serve Debug too, since linking nothing at all is worse. The vcpkg checkout is shared under $RBT_HOME/vcpkg rather than per project: a populated tree is large and slow to rebuild and belongs to no single project. The tool itself is pinned. It clones over https, not the ssh remote the old Actions/Vcpkg helper used - a CI runner or a fresh machine has no ssh key. Triplet comes from the manifest, defaulting to the host's. Restore necessarily runs before any build context exists, so a cross-compiled build has to state its triplet explicitly; the triplet is part of the pin, because two triplets of one port are genuinely different content. --PackageAdd writes a dependency and restores it in one invocation, --PackageRemove drops one. Specs are git:#, path:, url:# and vcpkg:#; the qualifier splits off the last '#' because a URL may contain one, and a 40-char hex qualifier is recorded as a commit rather than a tag. Edits go through the raw JSON so fields rbt does not model survive them, and empty fields are not written - a PackageDependency has one field per source kind and all but one are always empty. Tests cover the vcpkg installed-tree mapping against a faked tree (running vcpkg itself needs a large clone, a bootstrap and a network, none of which belongs in a suite that must pass on three CI hosts; the mapping is where the decisions are), and the spec grammar and manifest edits in full. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019nEaEVhDjn2dQC19BE9iaM --- Doc/ARCH.md | 1 + Doc/ARCH.zh-CN.md | 1 + Doc/HowToUse.md | 22 +- Doc/HowToUse.zh-CN.md | 19 +- .../Project/CppBuildProject.cs | 14 +- .../Project/PackageArgs.cs | 6 + .../Fetchers/VcpkgPackageFetcher.cs | 218 ++++++++++++++++++ .../PackageService/PackageManifest.cs | 8 +- .../PackageService/PackageManifestEditor.cs | 158 +++++++++++++ .../PackageService/PackageRestoreService.cs | 1 + .../TestPackageManifestEditor.cs | 173 ++++++++++++++ .../ReBuildTool.Test/TestPackageVcpkg.cs | 166 +++++++++++++ 12 files changed, 778 insertions(+), 9 deletions(-) create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs create mode 100644 ReBuildTool/ReBuildTool.Service/PackageService/PackageManifestEditor.cs create mode 100644 ReBuildTool/ReBuildTool.Test/TestPackageManifestEditor.cs create mode 100644 ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs diff --git a/Doc/ARCH.md b/Doc/ARCH.md index 957358e..1831932 100644 --- a/Doc/ARCH.md +++ b/Doc/ARCH.md @@ -145,6 +145,7 @@ Parse() │ ├─ IPackageFetcher per source Git (clone/fetch/reset) │ │ HttpArchive (download, sha256, unpack) │ │ Path (used in place) + │ │ Vcpkg (install, then describe as binary) │ └─ write RBTPackage.lock.json only when changed ├─ PackageModuleBinder synthesizes a rule for a binary package; │ installs a consumer-supplied overlay rule diff --git a/Doc/ARCH.zh-CN.md b/Doc/ARCH.zh-CN.md index 6de781d..2520bff 100644 --- a/Doc/ARCH.zh-CN.md +++ b/Doc/ARCH.zh-CN.md @@ -122,6 +122,7 @@ Parse() │ ├─ 按来源分派 IPackageFetcher Git（clone/fetch/reset） │ │ HttpArchive（下载、sha256 校验、解压） │ │ Path（原地使用） + │ │ Vcpkg（install 后描述为二进制包） │ └─ 写出 RBTPackage.lock.json 仅在内容变化时 ├─ PackageModuleBinder 为二进制包合成规则； │ 安装消费方提供的 overlay 规则 diff --git a/Doc/HowToUse.md b/Doc/HowToUse.md index 89b012b..6c92a7d 100644 --- a/Doc/HowToUse.md +++ b/Doc/HowToUse.md @@ -246,14 +246,16 @@ under `Packages/`, and records exactly what it resolved to in // a release archive, verified against its hash "zlib": { "url": "https://.../zlib-1.3.tar.gz", "sha256": "…", "strip": 1 }, // a directory on this machine, for local co-development - "LocalLib": { "path": "../LocalLib" } + "LocalLib": { "path": "../LocalLib" }, + // a vcpkg port + "fmt": { "vcpkg": "fmt", "triplet": "x64-windows" } } } ``` -Each dependency sets **exactly one** source (`git`, `url` or `path`), and a git -source must carry a `commit`, `tag` or `branch` — RBT resolves exact pins only -and will never pick a version for you. +Each dependency sets **exactly one** source (`git`, `url`, `path` or `vcpkg`), +and a git source must carry a `commit`, `tag` or `branch` — RBT resolves exact +pins only and will never pick a version for you. `url` accepts `.zip`, `.tar.gz`/`.tgz` and `.tar`. `strip` drops that many leading path components, like `tar --strip-components`, because release tarballs almost @@ -347,6 +349,18 @@ actually changes, so it does not churn your working tree. | `--Offline` | Never access the network. Fails if the lock is not already satisfied on disk. | | `--ForceRestore` | Re-fetch every package even when the lock is already satisfied. | | `--UpdateLock` | Re-resolve moving pins (tags and branches) and rewrite the lock, like `cargo update`. | +| `--PackageAdd =` | Write a dependency into `RBTPackage.json` and restore it in one go. | +| `--PackageRemove ` | Drop a dependency from `RBTPackage.json`. | + +`` is `git:#`, `path:`, `url:#` or +`vcpkg:#`. A 40-character hex qualifier is recorded as a commit, +anything else as a tag. + +```bash +rbt --Mode Restore --PackageAdd "GreeterLib=git:https://github.com/x/greeter.git#v1.2.0" +``` + +Edits go through the raw JSON, so any field RBT does not model survives them. ### Where things land diff --git a/Doc/HowToUse.zh-CN.md b/Doc/HowToUse.zh-CN.md index bf5a56c..b3e3a61 100644 --- a/Doc/HowToUse.zh-CN.md +++ b/Doc/HowToUse.zh-CN.md @@ -236,13 +236,15 @@ Target 规则不同：它的 `UsedModules` / `Plugins` 在任何 target `Setup` // 发布压缩包，按哈希校验 "zlib": { "url": "https://.../zlib-1.3.tar.gz", "sha256": "…", "strip": 1 }, // 本机上的目录，用于本地联调 - "LocalLib": { "path": "../LocalLib" } + "LocalLib": { "path": "../LocalLib" }, + // vcpkg port + "fmt": { "vcpkg": "fmt", "triplet": "x64-windows" } } } ``` -每条依赖**有且只有一个**来源（`git`、`url` 或 `path`）；git 来源必须带上 `commit`、 -`tag` 或 `branch` —— RBT 只接受精确 pin，永远不会替你挑版本。 +每条依赖**有且只有一个**来源（`git`、`url`、`path` 或 `vcpkg`）；git 来源必须带上 +`commit`、`tag` 或 `branch` —— RBT 只接受精确 pin，永远不会替你挑版本。 `url` 支持 `.zip`、`.tar.gz`/`.tgz` 和 `.tar`。`strip` 会丢掉指定数量的前导路径段， 等同于 `tar --strip-components` —— 因为发布用的 tarball 基本都会把内容包在一层 @@ -327,6 +329,17 @@ commit 不会 —— 这样后续 restore 能复现同一棵树，且完全不 | `--Offline` | 绝不访问网络。若 lock 尚未在磁盘上被满足则直接失败。 | | `--ForceRestore` | 即使 lock 已满足也重新拉取所有包。 | | `--UpdateLock` | 重新解析会移动的 pin（tag / branch）并重写 lock，相当于 `cargo update`。 | +| `--PackageAdd =` | 往 `RBTPackage.json` 写入一条依赖，并顺带 restore。 | +| `--PackageRemove ` | 从 `RBTPackage.json` 移除一条依赖。 | + +`` 的形式为 `git:#`、`path:<目录>`、`url:#` +或 `vcpkg:#`。40 位十六进制的限定符会被记为 commit，其余记为 tag。 + +```bash +rbt --Mode Restore --PackageAdd "GreeterLib=git:https://github.com/x/greeter.git#v1.2.0" +``` + +编辑走的是原始 JSON，因此 RBT 不认识的字段都会被原样保留。 ### 东西放在哪 diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs b/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs index a5ef0f5..5512832 100644 --- a/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs +++ b/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs @@ -206,7 +206,19 @@ public void RestorePackages() return; } - var result = service.Value.Restore(ProjectRoot, PackageArgs.Get().ToRestoreOptions()); + var packageArgs = PackageArgs.Get(); + // Manifest edits happen before resolution, so --PackageAdd both records the dependency and + // fetches it in one invocation. + if (packageArgs.PackageAdd.IsSet && !string.IsNullOrWhiteSpace(packageArgs.PackageAdd.Value)) + { + PackageManifestEditor.Add(ProjectRoot, packageArgs.PackageAdd.Value); + } + if (packageArgs.PackageRemove.IsSet && !string.IsNullOrWhiteSpace(packageArgs.PackageRemove.Value)) + { + PackageManifestEditor.Remove(ProjectRoot, packageArgs.PackageRemove.Value); + } + + var result = service.Value.Restore(ProjectRoot, packageArgs.ToRestoreOptions()); RestoredPackages.Clear(); RestoredPackages.AddRange(result.Packages); diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Project/PackageArgs.cs b/ReBuildTool/ReBuildTool.CppCompiler/Project/PackageArgs.cs index 2be60ba..df31337 100644 --- a/ReBuildTool/ReBuildTool.CppCompiler/Project/PackageArgs.cs +++ b/ReBuildTool/ReBuildTool.CppCompiler/Project/PackageArgs.cs @@ -23,6 +23,12 @@ public class PackageArgs : CommandLineArgGroup [CmdLine("re-resolve moving pins (tags and branches) and rewrite RBTPackage.lock.json")] public CmdLineArg UpdateLock { get; set; } = CmdLineArg.FromObject(nameof(UpdateLock), false); + [CmdLine("add a dependency to RBTPackage.json, e.g. MyLib=git:https://github.com/x/y.git#v1.0")] + public CmdLineArg PackageAdd { get; set; } + + [CmdLine("remove a dependency from RBTPackage.json by name")] + public CmdLineArg PackageRemove { get; set; } + public PackageRestoreOptions ToRestoreOptions() { return new PackageRestoreOptions diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs new file mode 100644 index 0000000..4cce6db --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs @@ -0,0 +1,218 @@ +using NiceIO; +using Newtonsoft.Json; +using ReBuildTool.Service.Global; +using ResetCore.Common; + +namespace ReBuildTool.Service.PackageService.Fetchers; + +/// +/// Bridges a vcpkg port into rbt's package model. +/// +/// vcpkg does the acquiring and building; this fetcher's job is translation. After +/// vcpkg install it writes an RBTPackage.json describing the installed tree as a +/// prebuilt binary package, so the ordinary binary-package path +/// (PackageModuleBinder / PackageArtifactSelector) takes it from there and nothing +/// downstream needs to know vcpkg exists. +/// +/// The vcpkg checkout is shared across projects under $RBT_HOME/vcpkg rather than kept per +/// project: a populated vcpkg tree is large and slow to rebuild, and it does not belong to any one +/// project's working tree. +/// +public class VcpkgPackageFetcher : IPackageFetcher +{ + /// + /// The vcpkg tool itself is pinned so that a restore is reproducible. Ports move independently; + /// this is the tooling, not the library. + /// + private const string VcpkgToolTag = "2024.02.14"; + + private const string VcpkgUrl = "https://github.com/microsoft/vcpkg.git"; + + public PackageSourceKind Kind => PackageSourceKind.Vcpkg; + + public static NPath VcpkgRoot => GlobalPaths.ReBuildToolHome.Combine("vcpkg"); + + public FetchedPackage Fetch(FetchRequest request) + { + var port = request.Dependency.Vcpkg!; + var triplet = request.Dependency.Triplet ?? DefaultTriplet(); + var installed = VcpkgRoot.Combine("installed", triplet); + var destination = request.DefaultDestination; + + var alreadyInstalled = installed.DirectoryExists() + && VcpkgRoot.Combine("installed", "vcpkg", "info").DirectoryExists(); + + if (!alreadyInstalled || request.Options.Force) + { + if (request.Options.Offline) + { + throw new PackageException( + $"--Offline was requested but vcpkg port \"{port}\" ({triplet}) is not installed yet. " + + $"Run a restore without --Offline first."); + } + EnsureBootstrapped(request); + Log.Info($"[package] vcpkg install {port}:{triplet}"); + ProcessRunner.RunOrThrow( + VcpkgExecutable().ToString(), + new[] { "install", $"{port}:{triplet}", "--recurse" }, + VcpkgRoot, + $"installing vcpkg port \"{port}\""); + } + + if (!installed.DirectoryExists()) + { + throw new PackageException( + $"vcpkg port \"{port}\" reported success but {installed} does not exist. " + + $"Is \"{triplet}\" a triplet this vcpkg supports?"); + } + + // The synthesized package is a directory holding nothing but a manifest; the headers and + // libraries stay where vcpkg put them and are referenced by absolute path. + destination.EnsureDirectoryExists(); + var manifest = DescribeInstalledTree(request.Name, port, installed); + WriteIfChanged(PackageManifest.PathIn(destination), manifest); + + return new FetchedPackage(destination, $"{port}:{triplet}"); + } + + /// + /// Renders a vcpkg installed tree as a binary-package manifest. + /// + /// Separated from the install so it can be exercised without a vcpkg checkout: the mapping (and + /// vcpkg's debug/release split) is the part with decisions in it, the install is just a + /// subprocess. + /// + public static string DescribeInstalledTree(string packageName, string port, NPath installed) + { + var manifest = new PackageManifest + { + Name = packageName, + Binary = new PackageBinarySpec + { + Module = packageName, + Includes = { installed.Combine("include").ToString() } + } + }; + + // vcpkg keeps the debug build in a parallel debug/ prefix. Mapping it to rbt's Debug + // configuration is the whole reason this is not a single artifact. + var release = LibrariesIn(installed.Combine("lib")); + var debug = LibrariesIn(installed.Combine("debug", "lib")); + + if (debug.Count > 0) + { + manifest.Binary.Artifacts.Add(new PackageBinaryArtifact + { + Config = "Debug", + LibraryDirectories = { installed.Combine("debug", "lib").ToString() }, + StaticLibraries = debug + }); + } + + if (release.Count > 0) + { + // One artifact per non-Debug configuration: an omitted config would also match Debug + // and both sets would be linked. + foreach (var configuration in new[] { "Release", "ReleasePlus", "ReleaseSize" }) + { + manifest.Binary.Artifacts.Add(new PackageBinaryArtifact + { + Config = configuration, + LibraryDirectories = { installed.Combine("lib").ToString() }, + StaticLibraries = release + }); + } + // When vcpkg produced no debug variant, the release one has to serve Debug too or a + // debug build would link nothing at all. + if (debug.Count == 0) + { + manifest.Binary.Artifacts.Add(new PackageBinaryArtifact + { + Config = "Debug", + LibraryDirectories = { installed.Combine("lib").ToString() }, + StaticLibraries = release + }); + } + } + + if (release.Count == 0 && debug.Count == 0) + { + Log.Info($"[package] vcpkg port \"{port}\" installed no libraries; treating it as header-only."); + } + + return JsonConvert.SerializeObject(manifest, Formatting.Indented) + Environment.NewLine; + } + + private static List LibrariesIn(NPath directory) + { + if (!directory.DirectoryExists()) + { + return new List(); + } + return directory.Files() + .Where(file => file.ExtensionWithDot is ".lib" or ".a") + .Select(file => file.FileName) + .OrderBy(name => name, StringComparer.Ordinal) + .ToList(); + } + + private static void EnsureBootstrapped(FetchRequest request) + { + if (!VcpkgRoot.Combine(".git").DirectoryExists()) + { + Log.Info($"[package] cloning vcpkg {VcpkgToolTag}"); + VcpkgRoot.EnsureParentDirectoryExists(); + // https rather than the ssh remote the old Actions/Vcpkg helper used: a CI runner or a + // fresh machine has no ssh key and would simply fail. + ProcessRunner.RunOrThrow( + "git", + new[] { "clone", "--branch", VcpkgToolTag, "--depth", "1", VcpkgUrl, VcpkgRoot.ToString() }, + null, + "cloning vcpkg"); + } + + if (VcpkgExecutable().FileExists()) + { + return; + } + + var bootstrap = VcpkgRoot.Combine( + PlatformHelper.IsWindows() ? "bootstrap-vcpkg.bat" : "bootstrap-vcpkg.sh"); + Log.Info("[package] bootstrapping vcpkg"); + ProcessRunner.RunOrThrow(bootstrap.ToString(), Array.Empty(), VcpkgRoot, "bootstrapping vcpkg"); + } + + private static NPath VcpkgExecutable() + { + return VcpkgRoot.Combine(PlatformHelper.IsWindows() ? "vcpkg.exe" : "vcpkg"); + } + + /// + /// Falls back to the host's triplet. rbt can cross-compile, and the triplet then has to be + /// stated explicitly with "triplet" - restore runs before a build context exists, so the + /// target platform is not knowable here. + /// + public static string DefaultTriplet() + { + var architecture = Environment.Is64BitOperatingSystem ? "x64" : "x86"; + if (PlatformHelper.IsWindows()) + { + return $"{architecture}-windows"; + } + if (PlatformHelper.IsOSX()) + { + return $"{architecture}-osx"; + } + return $"{architecture}-linux"; + } + + private static void WriteIfChanged(NPath path, string content) + { + if (path.FileExists() && path.ReadAllText() == content) + { + return; + } + path.EnsureParentDirectoryExists(); + path.WriteAllText(content); + } +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs index e91a8cc..16b8252 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs @@ -44,6 +44,12 @@ public class PackageDependency [JsonProperty("vcpkg")] public string? Vcpkg { get; set; } + /// + /// vcpkg triplet, e.g. x64-windows. Defaults to the host's. Restore runs before any + /// build context exists, so a cross-compiled build has to name the triplet explicitly. + /// + [JsonProperty("triplet")] public string? Triplet { get; set; } + [JsonProperty("version")] public string? Version { get; set; } /// @@ -113,7 +119,7 @@ public string PinKey(string packageName) PackageSourceKind.Git => $"git:{Git}@{GitRevision}", PackageSourceKind.HttpArchive => $"url:{Url}@{Sha256}", PackageSourceKind.Path => $"path:{Path}", - PackageSourceKind.Vcpkg => $"vcpkg:{Vcpkg}@{Version}", + PackageSourceKind.Vcpkg => $"vcpkg:{Vcpkg}@{Version}:{Triplet}", _ => throw new PackageException($"unknown source kind for package \"{packageName}\"") }; } diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifestEditor.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifestEditor.cs new file mode 100644 index 0000000..685248a --- /dev/null +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifestEditor.cs @@ -0,0 +1,158 @@ +using NiceIO; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using ResetCore.Common; + +namespace ReBuildTool.Service.PackageService; + +/// +/// Adds and removes dependencies in a project's RBTPackage.json from the command line. +/// +/// Edits go through rather than a round trip of the typed manifest so that +/// anything rbt does not model - comments aside, any field a newer rbt or a human added - survives +/// the edit instead of being silently dropped. +/// +public static class PackageManifestEditor +{ + /// + /// Parses the compact spec accepted by --PackageAdd: + /// git:<url>#<tag-or-commit>, path:<dir>, + /// url:<href>#<sha256> or vcpkg:<port>#<triplet>. + /// + public static PackageDependency ParseSpec(string spec) + { + var separator = spec.IndexOf(':'); + if (separator <= 0) + { + throw new PackageException( + $"cannot read package spec \"{spec}\": expected one of " + + $"git:#, path:, url:#, vcpkg:#."); + } + + var kind = spec.Substring(0, separator).ToLowerInvariant(); + var rest = spec.Substring(separator + 1); + + // Split on the LAST '#': a URL may legitimately contain one, the qualifier never does. + string? qualifier = null; + var hash = rest.LastIndexOf('#'); + if (hash >= 0) + { + qualifier = rest.Substring(hash + 1); + rest = rest.Substring(0, hash); + } + + switch (kind) + { + case "git": + if (qualifier == null) + { + throw new PackageException( + $"git spec \"{spec}\" pins no revision: write git:#. " + + $"rbt resolves exact pins only."); + } + // A 40-character hex string is a commit; anything else is a tag. Guessing wrong is + // harmless - both resolve to a commit in the lock - but this keeps the manifest honest. + return IsCommitSha(qualifier) + ? new PackageDependency { Git = rest, Commit = qualifier } + : new PackageDependency { Git = rest, Tag = qualifier }; + case "path": + return new PackageDependency { Path = rest }; + case "url": + return new PackageDependency { Url = rest, Sha256 = qualifier }; + case "vcpkg": + return new PackageDependency { Vcpkg = rest, Triplet = qualifier }; + default: + throw new PackageException( + $"unknown package source \"{kind}\" in \"{spec}\": expected git, path, url or vcpkg."); + } + } + + private static bool IsCommitSha(string value) + { + return value.Length == 40 && value.All(Uri.IsHexDigit); + } + + /// Adds or replaces one dependency. Returns true when the file changed. + public static bool Add(NPath projectRoot, string entry) + { + var separator = entry.IndexOf('='); + if (separator <= 0) + { + throw new PackageException( + $"cannot read --PackageAdd \"{entry}\": expected =, " + + $"for example MyLib=git:https://github.com/x/y.git#v1.0."); + } + + var name = entry.Substring(0, separator).Trim(); + var dependency = ParseSpec(entry.Substring(separator + 1).Trim()); + // Validate before writing: a manifest that cannot be resolved is worse than a rejected edit. + dependency.ResolveKind(name); + + var root = Load(projectRoot); + var dependencies = root["dependencies"] as JObject; + if (dependencies == null) + { + dependencies = new JObject(); + root["dependencies"] = dependencies; + } + + // Ignoring nulls and defaults keeps the written entry down to the fields that were actually + // set - a PackageDependency has one field per source kind, and all but one are empty. + var serializer = JsonSerializer.Create(new JsonSerializerSettings + { + NullValueHandling = NullValueHandling.Ignore, + DefaultValueHandling = DefaultValueHandling.Ignore + }); + dependencies[name] = JObject.FromObject(dependency, serializer); + Log.Info($"[package] added {name} = {dependency.PinKey(name)}"); + return Save(projectRoot, root); + } + + /// Removes one dependency. Returns true when the file changed. + public static bool Remove(NPath projectRoot, string name) + { + var root = Load(projectRoot); + if (root["dependencies"] is not JObject dependencies || dependencies.Remove(name) == false) + { + Log.Warning($"[package] {name} is not a dependency of this project; nothing to remove."); + return false; + } + + Log.Info($"[package] removed {name}"); + return Save(projectRoot, root); + } + + private static JObject Load(NPath projectRoot) + { + var path = PackageManifest.PathIn(projectRoot); + if (!path.FileExists()) + { + return new JObject { ["name"] = projectRoot.FileName }; + } + try + { + return JObject.Parse(path.ReadAllText()); + } + catch (JsonException e) + { + throw new PackageException($"{path} is not valid JSON: {e.Message}", e); + } + } + + private static bool Save(NPath projectRoot, JObject root) + { + var path = PackageManifest.PathIn(projectRoot); + // Null-valued fields come from the typed dependency's many optional sources; writing them + // out would bury the two that matter in a wall of nulls. + var content = JsonConvert.SerializeObject(root, Formatting.Indented, + new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore }) + + Environment.NewLine; + if (path.FileExists() && path.ReadAllText() == content) + { + return false; + } + path.EnsureParentDirectoryExists(); + path.WriteAllText(content); + return true; + } +} diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs index 9905bbe..60c37a1 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageRestoreService.cs @@ -50,6 +50,7 @@ private static IEnumerable CreateFetchers() yield return new GitPackageFetcher(); yield return new PathPackageFetcher(); yield return new HttpArchivePackageFetcher(); + yield return new VcpkgPackageFetcher(); } /// diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageManifestEditor.cs b/ReBuildTool/ReBuildTool.Test/TestPackageManifestEditor.cs new file mode 100644 index 0000000..24db420 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Test/TestPackageManifestEditor.cs @@ -0,0 +1,173 @@ +using NiceIO; +using ReBuildTool.Service.PackageService; + +namespace ReBuildTool.Test; + +/// +/// --PackageAdd / --PackageRemove: the spec grammar and the manifest edits. +/// +[TestFixture] +public class TestPackageManifestEditor +{ + private NPath WorkDirectory = null!; + + [SetUp] + public void SetUp() + { + WorkDirectory = Path.Combine(Path.GetTempPath(), $"rbt-editor-{Guid.NewGuid():N}") + .ToNPath() + .EnsureDirectoryExists(); + } + + [TearDown] + public void TearDown() + { + WorkDirectory.DeleteIfExists(DeleteMode.Normal); + } + + private PackageManifest Manifest() + { + return PackageManifest.ReadFrom(WorkDirectory)!; + } + + [Test] + public void AGitSpecWithATagIsParsed() + { + var dependency = PackageManifestEditor.ParseSpec("git:https://github.com/x/y.git#v1.2.0"); + + Assert.That(dependency.Git, Is.EqualTo("https://github.com/x/y.git")); + Assert.That(dependency.Tag, Is.EqualTo("v1.2.0")); + Assert.That(dependency.Commit, Is.Null); + } + + /// A full sha is recorded as a commit, not as a tag, so the manifest says what it means. + [Test] + public void AFortyCharacterHexQualifierIsTreatedAsACommit() + { + var sha = new string('a', 40); + + var dependency = PackageManifestEditor.ParseSpec($"git:https://github.com/x/y.git#{sha}"); + + Assert.That(dependency.Commit, Is.EqualTo(sha)); + Assert.That(dependency.Tag, Is.Null); + } + + /// The qualifier is split off the last '#', because a URL may contain one itself. + [Test] + public void AUrlContainingAHashIsSplitOnTheLastOne() + { + var dependency = PackageManifestEditor.ParseSpec("url:https://host/a#b/pkg.zip#abc123"); + + Assert.That(dependency.Url, Is.EqualTo("https://host/a#b/pkg.zip")); + Assert.That(dependency.Sha256, Is.EqualTo("abc123")); + } + + [Test] + public void PathAndVcpkgSpecsAreParsed() + { + Assert.That(PackageManifestEditor.ParseSpec("path:../Local").Path, Is.EqualTo("../Local")); + + var vcpkg = PackageManifestEditor.ParseSpec("vcpkg:fmt#x64-windows"); + Assert.That(vcpkg.Vcpkg, Is.EqualTo("fmt")); + Assert.That(vcpkg.Triplet, Is.EqualTo("x64-windows")); + } + + [Test] + public void AGitSpecWithoutARevisionIsRejected() + { + var exception = Assert.Throws( + () => PackageManifestEditor.ParseSpec("git:https://github.com/x/y.git")); + + Assert.That(exception!.Message, Does.Contain("exact pins")); + } + + [Test] + public void AnUnknownSourceIsRejected() + { + var exception = Assert.Throws( + () => PackageManifestEditor.ParseSpec("svn://somewhere")); + + Assert.That(exception!.Message, Does.Contain("svn")); + } + + [Test] + public void AddCreatesTheManifestWhenThereIsNone() + { + PackageManifestEditor.Add(WorkDirectory, "MyLib=git:https://github.com/x/y.git#v1.0"); + + var dependency = Manifest().Dependencies["MyLib"]; + Assert.That(dependency.Git, Is.EqualTo("https://github.com/x/y.git")); + Assert.That(dependency.Tag, Is.EqualTo("v1.0")); + } + + /// + /// The edit goes through the raw JSON so fields rbt does not model survive it - a manifest is + /// the user's file, not rbt's serialization format. + /// + [Test] + public void AddPreservesFieldsItDoesNotUnderstand() + { + PackageManifest.PathIn(WorkDirectory).WriteAllText( + "{ \"name\": \"Mine\", \"somethingElse\": { \"keep\": true }, " + + "\"dependencies\": { \"Existing\": { \"path\": \"../Existing\" } } }"); + + PackageManifestEditor.Add(WorkDirectory, "MyLib=path:../MyLib"); + + var text = PackageManifest.PathIn(WorkDirectory).ReadAllText(); + Assert.That(text, Does.Contain("somethingElse")); + Assert.That(text, Does.Contain("keep")); + Assert.That(Manifest().Dependencies.Keys, Is.EquivalentTo(new[] { "Existing", "MyLib" })); + } + + /// + /// A PackageDependency carries one field per source kind; writing the empty ones out would bury + /// the entry that matters in a wall of nulls. + /// + [Test] + public void AddDoesNotWriteEmptyFields() + { + PackageManifestEditor.Add(WorkDirectory, "MyLib=path:../MyLib"); + + var text = PackageManifest.PathIn(WorkDirectory).ReadAllText(); + Assert.That(text, Does.Not.Contain("null")); + Assert.That(text, Does.Not.Contain("\"git\"")); + Assert.That(text, Does.Not.Contain("\"strip\"")); + } + + [Test] + public void AddReplacesAnExistingEntry() + { + PackageManifestEditor.Add(WorkDirectory, "MyLib=git:https://github.com/x/y.git#v1.0"); + PackageManifestEditor.Add(WorkDirectory, "MyLib=git:https://github.com/x/y.git#v2.0"); + + Assert.That(Manifest().Dependencies["MyLib"].Tag, Is.EqualTo("v2.0")); + } + + [Test] + public void RemoveDropsTheEntry() + { + PackageManifestEditor.Add(WorkDirectory, "MyLib=path:../MyLib"); + PackageManifestEditor.Add(WorkDirectory, "Other=path:../Other"); + + Assert.That(PackageManifestEditor.Remove(WorkDirectory, "MyLib"), Is.True); + Assert.That(Manifest().Dependencies.Keys, Is.EquivalentTo(new[] { "Other" })); + } + + [Test] + public void RemovingSomethingAbsentIsNotAnError() + { + PackageManifestEditor.Add(WorkDirectory, "MyLib=path:../MyLib"); + + Assert.That(PackageManifestEditor.Remove(WorkDirectory, "NotThere"), Is.False); + Assert.That(Manifest().Dependencies.Keys, Is.EquivalentTo(new[] { "MyLib" })); + } + + [Test] + public void AMalformedAddIsRejectedWithAnExample() + { + var exception = Assert.Throws( + () => PackageManifestEditor.Add(WorkDirectory, "no-equals-sign")); + + Assert.That(exception!.Message, Does.Contain("=")); + } +} diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs b/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs new file mode 100644 index 0000000..25af267 --- /dev/null +++ b/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs @@ -0,0 +1,166 @@ +using NiceIO; +using ReBuildTool.Service.PackageService; +using ReBuildTool.Service.PackageService.Fetchers; + +namespace ReBuildTool.Test; + +/// +/// The vcpkg bridge's translation step: turning an installed tree into a binary-package manifest. +/// +/// Running vcpkg itself needs a large clone, a bootstrap and a network, none of which belongs in a +/// test suite that has to pass on three CI hosts. The mapping is where the decisions live - vcpkg's +/// debug/release split in particular - so the installed tree is faked and only the mapping is +/// exercised. +/// +[TestFixture] +public class TestPackageVcpkg +{ + private NPath WorkDirectory = null!; + + [SetUp] + public void SetUp() + { + WorkDirectory = Path.Combine(Path.GetTempPath(), $"rbt-vcpkg-{Guid.NewGuid():N}") + .ToNPath() + .EnsureDirectoryExists(); + } + + [TearDown] + public void TearDown() + { + WorkDirectory.DeleteIfExists(DeleteMode.Normal); + } + + /// Fakes the layout vcpkg produces under installed/<triplet>. + private NPath FakeInstalledTree(string[] releaseLibraries, string[] debugLibraries) + { + var installed = WorkDirectory.Combine("installed", "x64-linux"); + installed.Combine("include").EnsureDirectoryExists().Combine("thing.h").WriteAllText("#pragma once"); + foreach (var library in releaseLibraries) + { + installed.Combine("lib").EnsureDirectoryExists().Combine(library).WriteAllText(""); + } + foreach (var library in debugLibraries) + { + installed.Combine("debug", "lib").EnsureDirectoryExists().Combine(library).WriteAllText(""); + } + return installed; + } + + private static PackageManifest Describe(string name, NPath installed) + { + var json = VcpkgPackageFetcher.DescribeInstalledTree(name, "someport", installed); + return PackageManifest.Parse(json, "generated".ToNPath()); + } + + [Test] + public void TheIncludeDirectoryBecomesTheModulesPublicInclude() + { + var installed = FakeInstalledTree(new[] { "libthing.a" }, Array.Empty()); + + var manifest = Describe("Thing", installed); + + Assert.That(manifest.Binary, Is.Not.Null); + Assert.That(manifest.Binary!.Module, Is.EqualTo("Thing")); + Assert.That(manifest.Binary.Includes.Single(), Is.EqualTo(installed.Combine("include").ToString())); + } + + /// + /// vcpkg builds both variants into parallel prefixes. Linking the release libraries into a + /// Debug build (or both sets at once) is exactly the mistake this split exists to avoid. + /// + [Test] + public void DebugAndReleaseLibrariesMapToTheirOwnConfigurations() + { + var installed = FakeInstalledTree(new[] { "libthing.a" }, new[] { "libthingd.a" }); + + var manifest = Describe("Thing", installed); + + var debug = manifest.Binary!.Artifacts.Single(artifact => artifact.Config == "Debug"); + Assert.That(debug.StaticLibraries, Is.EqualTo(new[] { "libthingd.a" })); + Assert.That(debug.LibraryDirectories.Single(), + Is.EqualTo(installed.Combine("debug", "lib").ToString())); + + var release = manifest.Binary.Artifacts.Single(artifact => artifact.Config == "Release"); + Assert.That(release.StaticLibraries, Is.EqualTo(new[] { "libthing.a" })); + Assert.That(release.LibraryDirectories.Single(), Is.EqualTo(installed.Combine("lib").ToString())); + } + + [Test] + public void EveryReleaseConfigurationIsCovered() + { + var installed = FakeInstalledTree(new[] { "libthing.a" }, new[] { "libthingd.a" }); + + var manifest = Describe("Thing", installed); + + // An artifact with no config would also match Debug, so each one is named explicitly. + Assert.That( + manifest.Binary!.Artifacts.Select(artifact => artifact.Config).OrderBy(config => config), + Is.EqualTo(new[] { "Debug", "Release", "ReleasePlus", "ReleaseSize" })); + } + + /// + /// Plenty of ports build only a release variant. Leaving Debug uncovered would link nothing at + /// all in a debug build, which is far worse than using the release libraries. + /// + [Test] + public void ReleaseLibrariesServeDebugWhenThePortHasNoDebugBuild() + { + var installed = FakeInstalledTree(new[] { "libthing.a" }, Array.Empty()); + + var manifest = Describe("Thing", installed); + + var debug = manifest.Binary!.Artifacts.Single(artifact => artifact.Config == "Debug"); + Assert.That(debug.StaticLibraries, Is.EqualTo(new[] { "libthing.a" })); + Assert.That(debug.LibraryDirectories.Single(), Is.EqualTo(installed.Combine("lib").ToString())); + } + + [Test] + public void AHeaderOnlyPortProducesNoArtifacts() + { + var installed = FakeInstalledTree(Array.Empty(), Array.Empty()); + + var manifest = Describe("Thing", installed); + + Assert.That(manifest.Binary!.Artifacts, Is.Empty); + Assert.That(manifest.Binary.Includes, Is.Not.Empty); + } + + [Test] + public void NonLibraryFilesAreNotLinked() + { + var installed = FakeInstalledTree(new[] { "libthing.a" }, Array.Empty()); + // vcpkg drops pkg-config and cmake config files in lib/ alongside the real libraries. + installed.Combine("lib", "thing.pc").WriteAllText(""); + installed.Combine("lib", "pkgconfig").EnsureDirectoryExists(); + + var manifest = Describe("Thing", installed); + + Assert.That( + manifest.Binary!.Artifacts.SelectMany(artifact => artifact.StaticLibraries).Distinct(), + Is.EqualTo(new[] { "libthing.a" })); + } + + [Test] + public void TheHostTripletIsUsedWhenNoneIsDeclared() + { + var triplet = VcpkgPackageFetcher.DefaultTriplet(); + + Assert.That(triplet, Does.Match(@"^(x64|x86)-(windows|osx|linux)$")); + } + + [Test] + public void TheTripletIsPartOfThePin() + { + var manifest = PackageManifest.Parse( + "{ \"dependencies\": { \"fmt\": { \"vcpkg\": \"fmt\", \"triplet\": \"x64-windows\" }, " + + "\"fmt2\": { \"vcpkg\": \"fmt\", \"triplet\": \"arm64-osx\" } } }", + "test".ToNPath()); + + // Two triplets of one port are genuinely different content, so they must not look + // interchangeable to the conflict check. + Assert.That( + manifest.Dependencies["fmt"].PinKey("fmt"), + Is.Not.EqualTo(manifest.Dependencies["fmt2"].PinKey("fmt"))); + } +} From 62c85628af02beabdc986ef4e0127383c53219c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 14:01:26 +0000 Subject: [PATCH 04/10] fix(package): VS filters for out-of-tree modules; restore-only Restore Fixes the Windows CI failure and three points from review. The VS filter walk climbed from a module directory up to the project's Source folder. Only a module living under Source/ ever reaches that sentinel: a package's module sits under Packages/, and a package consumed through a path dependency is not under the project at all - so the walk ran off the top of the tree and NPath.FileName threw "not valid on a root level directory". It now stops at the project root and at the filesystem root, and a package outside the project is grouped under a Modules/ filter by its own directory name rather than being named relative to the project, which would have produced a "..\..\" filter Solution Explorer cannot display. This only ever fired on Windows because every other host defaults to the CMake generator; the new regression test forces VS generation explicitly, so it now covers all three. Restore no longer goes through Parse(). Parse() also compiles and loads the rule assembly and will scaffold a default Target/Module for a project that has none - neither belongs in "fetch the packages and write the lock, then stop", and the scaffolding in particular meant --Mode Restore could write source files into a bare checkout. IProjectInterface gains Restore() and Program.cs dispatches to it directly. vcpkg bootstrap runs through cmd.exe /c on Windows. A .bat is not an executable image, so CreateProcess cannot launch it and ProcessRunner does not use ShellExecute - as written it would have failed on every Windows host. An omitted vcpkg triplet now resolves to the host's before pins are compared. Previously { "vcpkg": "fmt" } and { "vcpkg": "fmt", "triplet": "" } produced different pin keys and would be reported as conflicting on the very machine where they are identical, and the lock recorded a pin that did not say which triplet was built. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019nEaEVhDjn2dQC19BE9iaM --- .../Project/CppBuildProject.cs | 4 +- .../VisualStudio/VCProject.Filter.cs | 78 +++++++---- .../CompileService/ProjectInterface.cs | 8 ++ .../Fetchers/VcpkgPackageFetcher.cs | 22 ++- .../PackageService/PackageManifest.cs | 12 +- .../TestPackageBuildIntegration.cs | 30 ++++ .../ReBuildTool.Test/TestPackageVcpkg.cs | 21 +++ .../ReBuildTool.Test/TestPackageVsFilters.cs | 132 ++++++++++++++++++ ReBuildTool/ReBuildTool/Program.cs | 12 +- 9 files changed, 284 insertions(+), 35 deletions(-) create mode 100644 ReBuildTool/ReBuildTool.Test/TestPackageVsFilters.cs diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs b/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs index 5512832..fef956c 100644 --- a/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs +++ b/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs @@ -184,7 +184,7 @@ class ${moduleName} public void Parse() { - RestorePackages(); + Restore(); ParseRules(); } @@ -197,7 +197,7 @@ public void Parse() /// A project without an RBTPackage.json pays nothing: the service returns immediately /// and no Packages/ directory or lock file is created. /// - public void RestorePackages() + public void Restore() { var service = ServiceContext.Instance.FindService(); if (!service) diff --git a/ReBuildTool/ReBuildTool.IDE/VisualStudio/VCProject.Filter.cs b/ReBuildTool/ReBuildTool.IDE/VisualStudio/VCProject.Filter.cs index d7d16b2..4a931be 100644 --- a/ReBuildTool/ReBuildTool.IDE/VisualStudio/VCProject.Filter.cs +++ b/ReBuildTool/ReBuildTool.IDE/VisualStudio/VCProject.Filter.cs @@ -128,38 +128,66 @@ private void GenerateUnassignedSourceFiles() private void GenerateModule(IModuleInterface moduleInterface) { - // generate all path filters - var path = moduleInterface.ModuleDirectory.ToNPath(); - while (path.FileName != InternalFilter.Source) + var moduleDirectory = moduleInterface.ModuleDirectory.ToNPath(); + + // Walk up from the module, one filter per ancestor, stopping at the project's Source + // folder. Only a module that lives under Source/ ever reaches that sentinel: a package's + // module sits under Packages/, and a package pulled in by path is not under the project at + // all. So the walk has to stop at the project root and at the filesystem root too - + // without those guards it runs off the top of the tree and NPath.FileName throws + // ("not valid on a root level directory"). + var path = moduleDirectory; + while (!path.IsRoot + && path.FileName != InternalFilter.Source + && path != cppSource.ProjectRoot + && path.IsChildOf(cppSource.ProjectRoot)) { - if (!AllFilters.ContainsKey(path.ToString())) - { - var filter = new Filter() - { - FilterName = path.RelativeTo(cppSource.ProjectRoot), - FilterGuid = Guid.NewGuid() - }; - AllFilters.Add(path, filter); - } - + GetOrAddFilter(path, moduleDirectory); path = path.Parent; } - - moduleInterface.ModuleDirectory.ToNPath().Files(true).ToList().ForEach(file => + + moduleDirectory.Files(true).ToList().ForEach(file => { - if (!AllFilters.TryGetValue(file.Parent, out var folderFilter)) - { - folderFilter = new Filter() - { - FilterName = file.Parent.RelativeTo(cppSource.ProjectRoot), - FilterGuid = Guid.NewGuid() - }; - AllFilters.Add(file.Parent, folderFilter); - } - folderFilter.Files.Add(file.RelativeTo(outputFolder)); + GetOrAddFilter(file.Parent, moduleDirectory).Files.Add(file.RelativeTo(outputFolder)); }); } + private Filter GetOrAddFilter(NPath directory, NPath moduleDirectory) + { + if (AllFilters.TryGetValue(directory, out var existing)) + { + return existing; + } + + var filter = new Filter + { + FilterName = FilterNameFor(directory, moduleDirectory), + FilterGuid = Guid.NewGuid() + }; + AllFilters.Add(directory, filter); + return filter; + } + + /// + /// The virtual folder a file appears under in Solution Explorer. It has to be a downward + /// path: anything inside the project is named relative to it, but a package consumed through + /// a path dependency lives outside the project entirely, and naming that relative to the + /// project root would yield a "..\..\" filter. Those are grouped under + /// by the package directory's own name instead. + /// + private string FilterNameFor(NPath directory, NPath moduleDirectory) + { + if (directory.IsChildOf(cppSource.ProjectRoot)) + { + return directory.RelativeTo(cppSource.ProjectRoot).ToString(); + } + + var moduleRoot = InternalFilter.Modules.ToNPath().Combine(moduleDirectory.FileName); + return directory == moduleDirectory + ? moduleRoot.ToString() + : moduleRoot.Combine(directory.RelativeTo(moduleDirectory)).ToString(); + } + private void FlushAllFilters() { foreach (var (key, filter) in AllFilters) diff --git a/ReBuildTool/ReBuildTool.Service/CompileService/ProjectInterface.cs b/ReBuildTool/ReBuildTool.Service/CompileService/ProjectInterface.cs index b6e2c53..08b79fd 100644 --- a/ReBuildTool/ReBuildTool.Service/CompileService/ProjectInterface.cs +++ b/ReBuildTool/ReBuildTool.Service/CompileService/ProjectInterface.cs @@ -4,6 +4,14 @@ namespace ReBuildTool.Service.CompileService; public interface IProjectInterface : IProvideByService { + /// + /// Fetch declared packages and write the lock, and nothing else. Kept separate from + /// , which also compiles and loads the rule assembly and will scaffold a + /// default Target/Module for a project that has none - side effects that have no business + /// happening during a cache-warm or an offline prep run. + /// + void Restore(); + void Parse(); void Setup(); void Build(string? targetName = null); diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs index 4cce6db..b6eb999 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs @@ -35,7 +35,7 @@ public class VcpkgPackageFetcher : IPackageFetcher public FetchedPackage Fetch(FetchRequest request) { var port = request.Dependency.Vcpkg!; - var triplet = request.Dependency.Triplet ?? DefaultTriplet(); + var triplet = request.Dependency.EffectiveTriplet; var installed = VcpkgRoot.Combine("installed", triplet); var destination = request.DefaultDestination; @@ -176,10 +176,24 @@ private static void EnsureBootstrapped(FetchRequest request) return; } - var bootstrap = VcpkgRoot.Combine( - PlatformHelper.IsWindows() ? "bootstrap-vcpkg.bat" : "bootstrap-vcpkg.sh"); Log.Info("[package] bootstrapping vcpkg"); - ProcessRunner.RunOrThrow(bootstrap.ToString(), Array.Empty(), VcpkgRoot, "bootstrapping vcpkg"); + if (PlatformHelper.IsWindows()) + { + // A .bat is not an executable image, so CreateProcess cannot launch it directly and + // ProcessRunner does not use ShellExecute. The interpreter has to be explicit. + ProcessRunner.RunOrThrow( + "cmd.exe", + new[] { "/c", VcpkgRoot.Combine("bootstrap-vcpkg.bat").ToString() }, + VcpkgRoot, + "bootstrapping vcpkg"); + return; + } + + ProcessRunner.RunOrThrow( + VcpkgRoot.Combine("bootstrap-vcpkg.sh").ToString(), + Array.Empty(), + VcpkgRoot, + "bootstrapping vcpkg"); } private static NPath VcpkgExecutable() diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs index 16b8252..7728ddf 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs @@ -108,6 +108,16 @@ public PackageSourceKind ResolveKind(string packageName) /// public string? GitRevision => Commit ?? Tag ?? Branch; + /// + /// The triplet a vcpkg dependency actually resolves to. An omitted means + /// the host's, so the default has to be applied here rather than only at fetch time - otherwise + /// { "vcpkg": "fmt" } and { "vcpkg": "fmt", "triplet": "<host>" } would look + /// like conflicting pins on the very machine where they are identical, and the lock would record + /// a pin that does not say which triplet was built. + /// + public string EffectiveTriplet => + string.IsNullOrWhiteSpace(Triplet) ? Fetchers.VcpkgPackageFetcher.DefaultTriplet() : Triplet; + /// /// Identity of this pin, used to detect conflicting declarations of the same package name /// coming from different manifests. Two dependencies with the same key are interchangeable. @@ -119,7 +129,7 @@ public string PinKey(string packageName) PackageSourceKind.Git => $"git:{Git}@{GitRevision}", PackageSourceKind.HttpArchive => $"url:{Url}@{Sha256}", PackageSourceKind.Path => $"path:{Path}", - PackageSourceKind.Vcpkg => $"vcpkg:{Vcpkg}@{Version}:{Triplet}", + PackageSourceKind.Vcpkg => $"vcpkg:{Vcpkg}@{Version}:{EffectiveTriplet}", _ => throw new PackageException($"unknown source kind for package \"{packageName}\"") }; } diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageBuildIntegration.cs b/ReBuildTool/ReBuildTool.Test/TestPackageBuildIntegration.cs index 9834797..e8eb071 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageBuildIntegration.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageBuildIntegration.cs @@ -88,6 +88,36 @@ private static void Build(NPath project) cppProject.Build(); } + /// + /// Restore has to do exactly that and stop. Routing it through Parse() would also compile and + /// load the rule assembly and - for a project that has no target yet - scaffold a default + /// Target/Module, which is not something a cache-warm or offline-prep run should write. + /// + [Test] + public void RestoreDoesNotCompileRulesOrScaffoldAProject() + { + var package = WorkDirectory.Combine("VendorPack").EnsureDirectoryExists(); + PackageManifest.PathIn(package).WriteAllText("{ \"name\": \"VendorPack\" }"); + + // Deliberately no Source/ at all: this is the state that would get scaffolded. + var project = WorkDirectory.Combine("Bare").EnsureDirectoryExists(); + PackageManifest.PathIn(project).WriteAllText( + "{ \"dependencies\": { \"VendorPack\": { \"path\": \"../VendorPack\" } } }"); + + CmdParser.Parse(); + ServiceContext.Instance.Init(); + var cppProject = ServiceContext.Instance.Create(project).Value; + cppProject.Restore(); + + // The packages are there and the lock was written... + Assert.That(PackageLockFile.ReadFrom(project), Is.Not.Null); + // ...and nothing else was. + Assert.That(project.Combine("Source").DirectoryExists(), Is.False, + "Restore must not scaffold a default project"); + Assert.That(project.Combine("Intermedia").DirectoryExists(), Is.False, + "Restore must not compile the rule assembly"); + } + /// /// A header-only prebuilt package: no rule of its own, so rbt synthesizes one, and the include /// path it declares has to reach the consuming module's compile line for this to link. diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs b/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs index 25af267..ee037df 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs @@ -149,6 +149,27 @@ public void TheHostTripletIsUsedWhenNoneIsDeclared() Assert.That(triplet, Does.Match(@"^(x64|x86)-(windows|osx|linux)$")); } + /// + /// An omitted triplet means the host's, so it has to be resolved before the pins are compared - + /// otherwise these two spellings of the same thing would be reported as a conflict on the very + /// machine where they are identical. + /// + [Test] + public void AnOmittedTripletPinsTheSameAsTheExplicitHostTriplet() + { + var host = VcpkgPackageFetcher.DefaultTriplet(); + var manifest = PackageManifest.Parse( + "{ \"dependencies\": { \"implicit\": { \"vcpkg\": \"fmt\" }, " + + $"\"explicit\": {{ \"vcpkg\": \"fmt\", \"triplet\": \"{host}\" }} }} }}", + "test".ToNPath()); + + Assert.That( + manifest.Dependencies["implicit"].PinKey("fmt"), + Is.EqualTo(manifest.Dependencies["explicit"].PinKey("fmt"))); + // And the recorded pin has to name the triplet that was actually built. + Assert.That(manifest.Dependencies["implicit"].PinKey("fmt"), Does.Contain(host)); + } + [Test] public void TheTripletIsPartOfThePin() { diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageVsFilters.cs b/ReBuildTool/ReBuildTool.Test/TestPackageVsFilters.cs new file mode 100644 index 0000000..e33ca1a --- /dev/null +++ b/ReBuildTool/ReBuildTool.Test/TestPackageVsFilters.cs @@ -0,0 +1,132 @@ +using NiceIO; +using ReBuildTool.Service.CompileService; +using ReBuildTool.Service.Context; +using ReBuildTool.Service.IDEService; +using ReBuildTool.ToolChain.Project; +using ResetCore.Common; + +namespace ReBuildTool.Test; + +/// +/// Visual Studio filter generation for modules that come from packages. +/// +/// The filter walk climbs from a module directory up to the project's Source folder. Only a +/// module living under Source/ ever reaches that sentinel - a package's module sits under +/// Packages/, and a package consumed through a path dependency is not under the project at +/// all - so without a stop at the project root the walk ran off the top of the tree and threw +/// "not valid on a root level directory". +/// +/// This only ever fired on Windows, because every other host defaults to the CMake generator. +/// +[TestFixture] +public class TestPackageVsFilters +{ + private NPath WorkDirectory = null!; + + [SetUp] + public void SetUp() + { + WorkDirectory = Path.Combine(Path.GetTempPath(), $"rbt-vsfilter-{Guid.NewGuid():N}") + .ToNPath() + .EnsureDirectoryExists(); + } + + [TearDown] + public void TearDown() + { + try + { + WorkDirectory.DeleteIfExists(DeleteMode.Normal); + } + catch (Exception) + { + // A leftover temp directory must never fail a test run. + } + } + + [Test] + public void APackageOutsideTheProjectDoesNotBreakFilterGeneration() + { + // The package lives beside the project, not inside it - a path dependency, the shape that + // has no ancestor in common with the project below the drive root. + var package = WorkDirectory.Combine("VendorPackage").EnsureDirectoryExists(); + PackageManifestFor(package); + package.Combine("VendorModule.module.cs").WriteAllText( + "using ReBuildTool.Service.CompileService;\n" + + "using ReBuildTool.ToolChain;\n" + + "public class VendorModule : CppModuleRule\n" + + "{\n" + + " public override void Setup(ICppBuildContext buildContext)\n" + + " {\n" + + " TargetBuildType = BuildType.StaticLibrary;\n" + + " PublicDefines.Add(\"VENDORMODULE_BUILT_AS_STATIC\");\n" + + " }\n" + + "}\n"); + package.Combine("Public").EnsureDirectoryExists().Combine("VendorModule.h").WriteAllText( + "#pragma once\nint vendor_answer();\n"); + package.Combine("Private").EnsureDirectoryExists().Combine("VendorModule.cpp").WriteAllText( + "#include \"VendorModule.h\"\nint vendor_answer() { return 42; }\n"); + + var project = WorkDirectory.Combine("Consumer").EnsureDirectoryExists(); + project.Combine("RBTPackage.json").WriteAllText( + "{ \"dependencies\": { \"VendorPackage\": { \"path\": \"../VendorPackage\" } } }"); + var source = project.Combine("Source").EnsureDirectoryExists(); + source.Combine("ConsumerTarget.target.cs").WriteAllText( + "using ReBuildTool.ToolChain;\n" + + "public class ConsumerTarget : CppTargetRule\n" + + "{\n" + + " public ConsumerTarget() { UsedModules.Add(\"ConsumerModule\"); }\n" + + "}\n"); + var module = source.Combine("ConsumerModule").EnsureDirectoryExists(); + module.Combine("ConsumerModule.module.cs").WriteAllText( + "using ReBuildTool.Service.CompileService;\n" + + "using ReBuildTool.ToolChain;\n" + + "public class ConsumerModule : CppModuleRule\n" + + "{\n" + + " public override void Setup(ICppBuildContext buildContext)\n" + + " {\n" + + " TargetBuildType = BuildType.Executable;\n" + + " Dependencies.Add(\"VendorModule\");\n" + + " }\n" + + "}\n"); + module.Combine("Public").EnsureDirectoryExists().Combine("ConsumerModule.h").WriteAllText( + "#pragma once\n"); + module.Combine("Private").EnsureDirectoryExists().Combine("ConsumerModule.cpp").WriteAllText( + "#include \"VendorModule.h\"\nint main() { return vendor_answer() == 42 ? 0 : 1; }\n"); + + CmdParser.Parse(); + ServiceContext.Instance.Init(); + ProjectGenArgs.Get().IDEProjectType.Value = ProjectGenType.VisualStudio; + + var cppProject = ServiceContext.Instance.Create(project).Value; + cppProject.Parse(); + Assert.DoesNotThrow(() => cppProject.Setup()); + + var filters = project.Combine("Intermedia/CppProject/VCProjects") + .Files("*.vcxproj.filters", true) + .ToList(); + Assert.That(filters, Is.Not.Empty, "a .vcxproj.filters should have been generated"); + + var text = filters.First().ReadAllText(); + // The out-of-project package is grouped under Modules/ rather than named with a "../../" + // filter, which is not something Solution Explorer can display. (File Include paths are + // relative to the output folder and legitimately contain "..", so only the Filter + // declarations are checked here.) + Assert.That(text.Replace('\\', '/'), Does.Contain("Modules/VendorPackage")); + + var filterNames = System.Text.RegularExpressions.Regex + .Matches(text, " match.Groups[1].Value) + .ToList(); + Assert.That(filterNames, Is.Not.Empty); + Assert.That(filterNames.Any(name => name.Replace('\\', '/').StartsWith("Modules/VendorPackage")), + Is.True, $"expected a Modules/VendorPackage filter, got: {string.Join(", ", filterNames)}"); + Assert.That(filterNames.Any(name => name.Contains("..")), Is.False, + $"filter names must not climb out of the project, got: {string.Join(", ", filterNames)}"); + } + + private static void PackageManifestFor(NPath package) + { + package.Combine("RBTPackage.json").WriteAllText("{ \"name\": \"VendorPackage\" }"); + } +} diff --git a/ReBuildTool/ReBuildTool/Program.cs b/ReBuildTool/ReBuildTool/Program.cs index 9d21e13..f1321e3 100644 --- a/ReBuildTool/ReBuildTool/Program.cs +++ b/ReBuildTool/ReBuildTool/Program.cs @@ -44,6 +44,15 @@ foreach (var project in projects) { + // Restore does not go through Parse(): Parse() also compiles and loads the rule assembly, + // and scaffolds a default Target/Module when the project has none. Neither belongs in + // "fetch the packages and write the lock, then stop". + if (command.Mode.Value == RunMode.Restore) + { + project.Restore(); + continue; + } + project.Parse(); switch (command.Mode.Value) { @@ -59,9 +68,6 @@ case RunMode.ReBuild: project.ReBuild(targetName); break; - case RunMode.Restore: - // Parse() already restored; this mode just stops before doing anything else. - break; default: break; } From 0890358bb8511d3c96618f421548db3a5245f589 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 14:13:34 +0000 Subject: [PATCH 05/10] docs(package): correct stale comments; tidy the test server construction Three comments no longer matched the code they describe: - CppBuildProject's RestoredPackages cref still pointed at RestorePackages, renamed to Restore() in the previous commit, so the cref no longer resolved. - PackageLock.Resolved and FetchedPackage.Resolved both said a path dependency records an absolute path. It records the path exactly as declared - deliberately, since the lock is committed and an absolute path would be meaningless in any other checkout. Both now say what the field actually holds for each source. LocalServer in the archive tests probed for a free port by assigning the readonly field inside the loop and then null-checking a non-nullable field. It compiled (definite assignment is not enforced for class fields) but read as though it might not have. The probe now uses a local and the fields are assigned once. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019nEaEVhDjn2dQC19BE9iaM --- .../Project/CppBuildProject.cs | 2 +- .../Fetchers/IPackageFetcher.cs | 7 +++++- .../PackageService/PackageLock.cs | 6 ++++- .../ReBuildTool.Test/TestPackageArchive.cs | 23 +++++++++++-------- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs b/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs index fef956c..83dcd4f 100644 --- a/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs +++ b/ReBuildTool/ReBuildTool.CppCompiler/Project/CppBuildProject.cs @@ -605,7 +605,7 @@ private void PostCompile(CppBuilder builder) private Dictionary TargetRulePaths { get; } = new(); private Dictionary ModuleRulePaths { get; } = new(); - /// Packages materialized by the last , in dependency order. + /// Packages materialized by the last , in dependency order. private List RestoredPackages { get; } = new(); /// diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs index 9ff1942..1649f63 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs @@ -49,7 +49,12 @@ public FetchedPackage(NPath root, string resolved) /// Where the package content lives. For a path dependency this is outside Packages/. public NPath Root { get; } - /// What the pin actually resolved to: a commit sha, an archive hash, or an absolute path. + /// + /// What the pin actually resolved to, and what the lock records: a commit sha for git, an + /// archive sha256 for a URL, port:triplet for vcpkg. A path dependency reports the path + /// as declared rather than - the lock is committed and shared, so it must + /// not carry a location that is only meaningful on the machine that wrote it. + /// public string Resolved { get; } } diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageLock.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageLock.cs index c96a983..ea4719c 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageLock.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageLock.cs @@ -17,7 +17,11 @@ public class LockedPackage /// The git/http URL, or the declared path for a path dependency. [JsonProperty("origin")] public string? Origin { get; set; } - /// Commit sha for git, content sha256 for an archive, the absolute path for a path dependency. + /// + /// Commit sha for git, archive sha256 for a URL, port:triplet for vcpkg. For a path + /// dependency it is the path exactly as declared, not where it landed on this machine - an + /// absolute path would make the committed lock useless to every other checkout. + /// [JsonProperty("resolved")] public string? Resolved { get; set; } /// The pin this entry was produced from, so a changed manifest invalidates the lock. diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs b/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs index 4ada9bc..c2b7987 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs @@ -87,27 +87,30 @@ private sealed class LocalServer : IDisposable public LocalServer(NPath file) { - // Port 0 is not available through HttpListener, so probe upward for a free one. - for (var port = 18800; port < 18900; port++) + // Port 0 is not available through HttpListener, so probe upward for a free one. The + // probe writes to a local and the fields are assigned once, so neither is ever observed + // half-initialized. + HttpListener? started = null; + var url = string.Empty; + for (var port = 18800; port < 18900 && started == null; port++) { var listener = new HttpListener(); listener.Prefixes.Add($"http://127.0.0.1:{port}/"); try { listener.Start(); - Listener = listener; - Url = $"http://127.0.0.1:{port}/{file.FileName}"; - break; + started = listener; + url = $"http://127.0.0.1:{port}/{file.FileName}"; } catch (HttpListenerException) { listener.Close(); } } - if (Listener == null) - { - throw new InvalidOperationException("no free loopback port for the test server"); - } + + Listener = started + ?? throw new InvalidOperationException("no free loopback port for the test server"); + Url = url; var bytes = File.ReadAllBytes(file.ToString()); Task.Run(() => @@ -130,7 +133,7 @@ public LocalServer(NPath file) }); } - public string Url { get; } = string.Empty; + public string Url { get; } public void Dispose() { From 2127d5b977c32524a3717a1908159e84834196b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 14:30:18 +0000 Subject: [PATCH 06/10] fix(package): validate names from manifests; key archive cache on its URL Three findings from review, two of them about trusting a manifest too far. A package fetched from a remote declares its own transitive dependencies, so the names arriving from it are exactly as trustworthy as the package is - and rbt turns them into filesystem paths and into generated C# it then compiles and runs. Package names are now validated before they are combined into a path. The check sits in the resolver, so it covers every name in the walk rather than only the ones in the project's own manifest, and is re-applied in FetchRequest for any future caller building a request directly. A dependency keyed "../outside" would previously have placed the package next to Packages/ instead of inside it; the sidecar file the archive fetcher writes had the same hole and now goes through the same containment check. Binary-package module names are validated as plain C# identifiers. The name is interpolated into "public class " in a generated rule that becomes part of CompileRules.dll, so a name carrying punctuation could have closed the declaration and appended arbitrary code to an assembly rbt executes during the build. Rejected rather than escaped: a package still has no business naming a class outside its own. The archive fetcher's cache hit is keyed on the origin URL when the manifest gives no sha256. Previously any stamp satisfied that case, so editing a dependency's url left the previous archive unpacked on disk while the lock recorded the new origin - stale content under a fresh-looking pin. The stamp now carries the URL alongside the hash; a stamp from an older rbt simply misses once and re-downloads. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019nEaEVhDjn2dQC19BE9iaM --- .../Package/PackageModuleBinder.cs | 8 ++- .../Fetchers/HttpArchivePackageFetcher.cs | 34 ++++++++++--- .../Fetchers/IPackageFetcher.cs | 34 ++++++++++++- .../PackageService/PackageManifest.cs | 50 +++++++++++++++++++ .../PackageService/PackageResolver.cs | 5 ++ .../ReBuildTool.Test/TestPackageArchive.cs | 35 +++++++++++++ .../TestPackageBinaryModule.cs | 23 +++++++++ .../ReBuildTool.Test/TestPackageResolver.cs | 36 +++++++++++++ 8 files changed, 217 insertions(+), 8 deletions(-) diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs b/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs index f6c02dc..cfde56a 100644 --- a/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs +++ b/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs @@ -53,7 +53,13 @@ public static List Bind(NPath packagesRoot, IEnumerable return null; } - var moduleName = string.IsNullOrWhiteSpace(binary.Module) ? package.Name : binary.Module!; + // The name comes out of the package's own manifest, which for a remote package is not the + // consuming project's to trust. It is interpolated into a directory name and into the + // "public class " of a generated rule that rbt then compiles and runs, so anything + // other than a plain identifier is rejected rather than escaped. + var moduleName = PackageNames.ValidateModuleName( + string.IsNullOrWhiteSpace(binary.Module) ? package.Name : binary.Module!, + package.Name); var moduleDirectory = packagesRoot.Combine(GeneratedFolderName, moduleName); // The framework unconditionally registers a module's Public/ and Private/ directories; the // source globbing warns once per missing path, so create them rather than emit noise. diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/HttpArchivePackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/HttpArchivePackageFetcher.cs index 82f9a70..cf9e56c 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/HttpArchivePackageFetcher.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/HttpArchivePackageFetcher.cs @@ -15,7 +15,10 @@ namespace ReBuildTool.Service.PackageService.Fetchers; /// public class HttpArchivePackageFetcher : IPackageFetcher { - /// Records which archive the extracted directory came from. Kept beside the package, not inside it. + /// + /// Records which archive the extracted directory came from - the hash on the first line, the + /// URL it came from on the second. Kept beside the package, not inside it. + /// private const string StampFileName = ".rbt-archive-sha256"; public PackageSourceKind Kind => PackageSourceKind.HttpArchive; @@ -24,17 +27,23 @@ public FetchedPackage Fetch(FetchRequest request) { var url = request.Dependency.Url!; var destination = request.DefaultDestination; - var stamp = request.PackagesRoot.Combine($"{request.Name}{StampFileName}"); + var stamp = request.SidecarFile(StampFileName); var expected = request.Dependency.Sha256; // Already unpacked from the very archive the manifest asks for: nothing to do, and no // reason to touch the network. if (!request.Options.Force && destination.DirectoryExists() && stamp.FileExists()) { - var current = stamp.ReadAllText().Trim(); - if (expected == null || Hashing.Matches(expected, current)) + var (stampedHash, stampedUrl) = ReadStamp(stamp); + // With a sha256 the hash decides. Without one there is nothing to compare the content + // against, so the URL has to: otherwise editing the manifest's url would leave the old + // tree on disk while the lock recorded the new origin. + var satisfied = expected != null + ? Hashing.Matches(expected, stampedHash) + : stampedUrl == url; + if (satisfied) { - return new FetchedPackage(destination, current); + return new FetchedPackage(destination, stampedHash); } } @@ -87,7 +96,20 @@ public FetchedPackage Fetch(FetchRequest request) download.DeleteIfExists(); } - stamp.WriteAllText(actual); + stamp.WriteAllText($"{actual}{Environment.NewLine}{url}{Environment.NewLine}"); return new FetchedPackage(destination, actual); } + + /// + /// Reads the hash and origin URL back out of the stamp. A stamp written by an older rbt has + /// only the hash; it reports an empty URL, which simply makes the no-sha256 fast path miss and + /// re-download once. + /// + private static (string Hash, string Url) ReadStamp(NPath stamp) + { + var lines = stamp.ReadAllLines(); + return ( + lines.Length > 0 ? lines[0].Trim() : string.Empty, + lines.Length > 1 ? lines[1].Trim() : string.Empty); + } } diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs index 1649f63..8ba1737 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/IPackageFetcher.cs @@ -35,7 +35,39 @@ public FetchRequest( /// The matching lock entry, when the project already has one. public LockedPackage? Locked { get; } - public NPath DefaultDestination => PackagesRoot.Combine(Name); + /// + /// Where a fetcher materializes this package. The name is validated by the resolver before the + /// request is built; it is re-checked here so that any future caller constructing a request + /// directly cannot land a package outside . + /// + public NPath DefaultDestination + { + get + { + var destination = PackagesRoot.Combine(PackageNames.ValidatePackageName(Name)); + if (!destination.IsChildOf(PackagesRoot)) + { + throw new PackageException( + $"package \"{Name}\" would be placed at \"{destination}\", outside \"{PackagesRoot}\"."); + } + return destination; + } + } + + /// + /// Bookkeeping file a fetcher may keep for this package. Lives beside the package rather than + /// inside it, and is subject to the same containment rule as . + /// + public NPath SidecarFile(string suffix) + { + var sidecar = PackagesRoot.Combine($"{PackageNames.ValidatePackageName(Name)}{suffix}"); + if (!sidecar.IsChildOf(PackagesRoot)) + { + throw new PackageException( + $"package \"{Name}\" would write \"{sidecar}\", outside \"{PackagesRoot}\"."); + } + return sidecar; + } } public class FetchedPackage diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs index 7728ddf..71d5fe7 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs @@ -236,6 +236,56 @@ public static PackageManifest Parse(string json, NPath origin) } } +/// +/// Names that come out of a manifest are attacker-controlled in the same sense the manifest is: a +/// package fetched from a remote declares its own transitive dependencies, and rbt turns those +/// names into filesystem paths and, for a binary package, into generated C# that the build then +/// compiles and runs. Both uses have to be gated. +/// +public static class PackageNames +{ + // Deliberately narrow. Anything outside this set has no legitimate use in a package name and + // is exactly what a traversal ("../..") or an injection would need. + private static readonly System.Text.RegularExpressions.Regex PackageNamePattern = + new("^[A-Za-z0-9][A-Za-z0-9._-]*$", System.Text.RegularExpressions.RegexOptions.Compiled); + + // A generated rule declares "public class ", so the name has to be a plain C# identifier. + private static readonly System.Text.RegularExpressions.Regex IdentifierPattern = + new("^[A-Za-z_][A-Za-z0-9_]*$", System.Text.RegularExpressions.RegexOptions.Compiled); + + /// + /// Validates a package name before it is combined into a path. Rejects separators, "." and + /// ".." - a dependency keyed "../outside" would otherwise place the package next to + /// Packages/ rather than inside it. + /// + public static string ValidatePackageName(string name) + { + if (string.IsNullOrWhiteSpace(name) || !PackageNamePattern.IsMatch(name)) + { + throw new PackageException( + $"\"{name}\" is not a usable package name. Names may contain letters, digits, " + + $"'.', '_' and '-', and must start with a letter or digit."); + } + return name; + } + + /// + /// Validates a name that will be emitted into generated C# source. Beyond path safety, a name + /// carrying punctuation could close the class declaration and append arbitrary code to the + /// rule assembly - which rbt compiles and executes as part of the build. + /// + public static string ValidateModuleName(string name, string packageName) + { + if (string.IsNullOrWhiteSpace(name) || !IdentifierPattern.IsMatch(name)) + { + throw new PackageException( + $"package \"{packageName}\" declares the module name \"{name}\", which is not a valid " + + $"C# identifier. rbt generates a rule class from it, so it must be a plain identifier."); + } + return name; + } +} + public class PackageException : Exception { public PackageException(string message) : base(message) diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs index 07f365d..da94f4b 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs @@ -74,6 +74,11 @@ public PackageRestoreResult Resolve( private void ResolveOne(string name, PackageDependency declared, NPath declaringDirectory) { + // Checked here rather than in each fetcher: a package fetched from a remote declares its + // own dependencies, so every name reaching this walk - not just the ones in the project's + // own manifest - becomes a directory under Packages/. + PackageNames.ValidatePackageName(name); + var dependency = Overrides.TryGetValue(name, out var overridden) ? overridden : declared; var pinKey = dependency.PinKey(name); diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs b/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs index c2b7987..3a7aef5 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageArchive.cs @@ -233,6 +233,41 @@ public void AChecksumMismatchIsFatalAndExplainsItself() Assert.That(project.Combine("Packages", "Geo").DirectoryExists(), Is.False); } + /// + /// Without a sha256 there is nothing to compare the unpacked tree against, so the cache hit has + /// to be keyed on the URL. Otherwise editing the manifest's url leaves the previous archive on + /// disk while the lock records the new origin - stale content under a fresh-looking pin. + /// + [Test] + public void ChangingTheUrlWithNoChecksumStillReplacesTheContent() + { + var first = CreateZip("first.zip", "pkg-1.0", ("first.txt", "1")); + var second = CreateZip("second.zip", "pkg-2.0", ("second.txt", "2")); + + NPath project; + using (var server = new LocalServer(first)) + { + project = CreateProject( + "{ \"dependencies\": { \"Geo\": { " + + $"\"url\": \"{server.Url}\", \"strip\": 1 }} }} }}"); + new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + } + Assert.That(project.Combine("Packages", "Geo", "first.txt").FileExists(), Is.True); + + using (var server = new LocalServer(second)) + { + PackageManifest.PathIn(project).WriteAllText( + "{ \"dependencies\": { \"Geo\": { " + + $"\"url\": \"{server.Url}\", \"strip\": 1 }} }} }}"); + new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + } + + Assert.That(project.Combine("Packages", "Geo", "second.txt").FileExists(), Is.True, + "the new archive should have been fetched and unpacked"); + Assert.That(project.Combine("Packages", "Geo", "first.txt").FileExists(), Is.False, + "the previous archive's content should be gone"); + } + [Test] public void ASecondRestoreOfAnArchivePackageNeedsNoNetwork() { diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs b/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs index 7e33af7..37e7e5d 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs @@ -104,6 +104,29 @@ public void TheGeneratedRuleIsPlatformIndependentAndDoesNotChurn() Assert.That(File.GetLastWriteTimeUtc(generated), Is.EqualTo(writtenAt)); } + /// + /// The module name is interpolated into a directory name and into the "public class <name>" + /// of a rule that rbt compiles and executes. A remote package's manifest is not the consuming + /// project's to trust, so anything but a plain identifier has to be refused - escaping it would + /// still leave a package able to name a class it has no business naming. + /// + [TestCase("../escape")] + [TestCase("Evil { } public class Injected : CppModuleRule { //")] + [TestCase("has space")] + [TestCase("1StartsWithDigit")] + public void AModuleNameThatIsNotAPlainIdentifierIsRejected(string moduleName) + { + var package = BinaryPackage("PrebuiltPack", + $"{{ \"module\": \"{moduleName.Replace("\\", "\\\\").Replace("\"", "\\\"")}\", \"artifacts\": [] }}"); + var packagesRoot = WorkDirectory.Combine("Packages"); + + var exception = Assert.Throws( + () => PackageModuleBinder.Bind(packagesRoot, new[] { package })); + + Assert.That(exception!.Message, Does.Contain("PrebuiltPack")); + Assert.That(exception.Message, Does.Contain("identifier")); + } + [Test] public void AnOverlayIsCopiedIntoThePackageItDescribes() { diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageResolver.cs b/ReBuildTool/ReBuildTool.Test/TestPackageResolver.cs index a6f967a..d191541 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageResolver.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageResolver.cs @@ -154,6 +154,42 @@ public void AnOverrideResolvesAConflict() Assert.That(shared.Root.FileName, Is.EqualTo("CopyOne")); } + /// + /// A package fetched from a remote declares its own dependencies, so the names reaching this + /// walk are no more trustworthy than the manifest they came from - and every one of them + /// becomes a directory under Packages/. + /// + [TestCase("../outside")] + [TestCase("..")] + [TestCase("nested/name")] + [TestCase("back\\slash")] + public void APackageNameThatEscapesThePackagesDirectoryIsRejected(string name) + { + WorkDirectory.Combine("outside").EnsureDirectoryExists(); + var root = PackageManifest.Parse( + $"{{ \"dependencies\": {{ \"{name.Replace("\\", "\\\\")}\": {{ \"path\": \"../outside\" }} }} }}", + WorkDirectory.Combine("root", PackageManifest.FileName)); + var rootDirectory = WorkDirectory.Combine("root").EnsureDirectoryExists(); + + var exception = Assert.Throws(() => Resolve(root, rootDirectory)); + + Assert.That(exception!.Message, Does.Contain("not a usable package name")); + } + + [TestCase("Fine")] + [TestCase("with.dots")] + [TestCase("with-dash_and_underscore")] + [TestCase("0leading-digit")] + public void OrdinaryPackageNamesAreAccepted(string name) + { + WritePackage(name); + var rootDirectory = WorkDirectory.Combine("root").EnsureDirectoryExists(); + + var result = Resolve(RootDependingOn(name), rootDirectory); + + Assert.That(result.Packages.Single().Name, Is.EqualTo(name)); + } + [Test] public void MissingPathDependencyNamesWhatItLookedFor() { From e65af7dafd45f7d05af77a02a49d1e0f82ddf13a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 14:40:06 +0000 Subject: [PATCH 07/10] fix(package): reject colliding binary module names; drop the inert vcpkg version Two binary packages could both name their module the same thing. Both generate to Packages/.generated//.module.cs - one file - so the second write won and the build quietly depended on which package was processed last. The binder now tracks the names it has generated and reports both culprits. The same clash between two source packages was already caught in ParseRules, where they are two distinct files claiming one name; this path had to catch its own. PackageDependency.Version is removed. Nothing ever read it: which version of a vcpkg port you get is decided by the vcpkg checkout, which the fetcher pins to a fixed tag, and real per-port pinning would need vcpkg manifest mode and a versioning baseline that this bridge does not set up. Keeping the field would have left a knob that silently does nothing, and it also made every vcpkg pin read "vcpkg:fmt@:x64-windows" in lock files and conflict messages. Pins are now "vcpkg::". The restore result's ordering comment claimed deepest-first. It is not: a package is recorded before its own dependencies are walked, so it precedes them. Nothing downstream depends on the order - the packages become rule-glob roots and the lock is sorted by name on write - so the comment now says what actually holds rather than promising a guarantee that was never provided. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019nEaEVhDjn2dQC19BE9iaM --- .../Package/PackageModuleBinder.cs | 22 +++++++++++-- .../PackageService/PackageManifest.cs | 7 ++-- .../PackageService/PackageResolver.cs | 5 +-- .../TestPackageBinaryModule.cs | 32 +++++++++++++++++++ .../ReBuildTool.Test/TestPackageVcpkg.cs | 15 +++++++++ 5 files changed, 75 insertions(+), 6 deletions(-) diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs b/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs index cfde56a..c768a3a 100644 --- a/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs +++ b/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageModuleBinder.cs @@ -32,9 +32,15 @@ public static class PackageModuleBinder public static List Bind(NPath packagesRoot, IEnumerable packages) { var roots = new List(); + // Two binary packages are free to pick the same binary.module name, and both would generate + // to Packages/.generated//.module.cs - the same file. The second write would + // simply win, leaving the build silently dependent on which package happened to come last. + // (A collision between two *source* packages is caught later, in ParseRules, because those + // are two distinct files claiming one module name. This path has to catch its own.) + var generatedBy = new Dictionary(); foreach (var package in packages) { - var generated = BindOne(packagesRoot, package); + var generated = BindOne(packagesRoot, package, generatedBy); if (generated != null) { roots.Add(generated); @@ -43,7 +49,10 @@ public static List Bind(NPath packagesRoot, IEnumerable return roots; } - private static NPath? BindOne(NPath packagesRoot, RestoredPackage package) + private static NPath? BindOne( + NPath packagesRoot, + RestoredPackage package, + Dictionary generatedBy) { InstallOverlay(package); @@ -60,6 +69,15 @@ public static List Bind(NPath packagesRoot, IEnumerable var moduleName = PackageNames.ValidateModuleName( string.IsNullOrWhiteSpace(binary.Module) ? package.Name : binary.Module!, package.Name); + + if (generatedBy.TryGetValue(moduleName, out var owner)) + { + throw new Exception( + $"packages \"{owner}\" and \"{package.Name}\" both provide a module named " + + $"\"{moduleName}\". Only one of them can, since a module is depended on by name - " + + $"set a distinct \"module\" in one package's {PackageManifest.FileName}."); + } + generatedBy.Add(moduleName, package.Name); var moduleDirectory = packagesRoot.Combine(GeneratedFolderName, moduleName); // The framework unconditionally registers a module's Public/ and Private/ directories; the // source globbing warns once per missing path, so create them rather than emit noise. diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs index 71d5fe7..6700009 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs @@ -50,7 +50,10 @@ public class PackageDependency /// [JsonProperty("triplet")] public string? Triplet { get; set; } - [JsonProperty("version")] public string? Version { get; set; } + // No "version" for a vcpkg dependency: which version of a port you get is decided by the vcpkg + // checkout, which VcpkgPackageFetcher pins to a fixed tag. Accepting a per-port version here + // would be a knob that silently does nothing - real per-port pinning needs vcpkg manifest mode + // and a versioning baseline, which this bridge does not set up. /// /// Path (relative to the manifest that declares this dependency) of a .module.cs to @@ -129,7 +132,7 @@ public string PinKey(string packageName) PackageSourceKind.Git => $"git:{Git}@{GitRevision}", PackageSourceKind.HttpArchive => $"url:{Url}@{Sha256}", PackageSourceKind.Path => $"path:{Path}", - PackageSourceKind.Vcpkg => $"vcpkg:{Vcpkg}@{Version}:{EffectiveTriplet}", + PackageSourceKind.Vcpkg => $"vcpkg:{Vcpkg}:{EffectiveTriplet}", _ => throw new PackageException($"unknown source kind for package \"{packageName}\"") }; } diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs index da94f4b..ac02ddd 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs @@ -67,8 +67,9 @@ public PackageRestoreResult Resolve( Packages = Resolved.Values.Select(entry => entry.Locked).ToList() }; - // Deepest-first: a package is listed after everything it needed, which is the order a - // reader wants and costs nothing to produce here. + // Discovery order, not dependency order: a package is recorded before its own dependencies + // are walked, so it precedes them here. Nothing downstream needs a topological order - the + // packages become rule-glob roots, and the lock is sorted by name when it is written. return new PackageRestoreResult(Resolved.Values.Select(entry => entry.Package).ToList()); } diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs b/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs index 37e7e5d..23578d6 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs @@ -127,6 +127,38 @@ public void AModuleNameThatIsNotAPlainIdentifierIsRejected(string moduleName) Assert.That(exception.Message, Does.Contain("identifier")); } + /// + /// Both packages would generate to Packages/.generated/<module>/<module>.module.cs - one file. + /// Left alone the second write wins and the build quietly depends on processing order, so the + /// collision has to be reported instead. (The same clash between two source packages is caught + /// later in ParseRules, where they are two distinct files claiming one name.) + /// + [Test] + public void TwoBinaryPackagesClaimingOneModuleNameIsAnError() + { + var first = BinaryPackage("FirstPack", "{ \"module\": \"SharedName\", \"artifacts\": [] }"); + var second = BinaryPackage("SecondPack", "{ \"module\": \"SharedName\", \"artifacts\": [] }"); + + var exception = Assert.Throws( + () => PackageModuleBinder.Bind(WorkDirectory.Combine("Packages"), new[] { first, second })); + + // Both culprits have to be named, or the user has no idea which two packages to look at. + Assert.That(exception!.Message, Does.Contain("FirstPack")); + Assert.That(exception.Message, Does.Contain("SecondPack")); + Assert.That(exception.Message, Does.Contain("SharedName")); + } + + [Test] + public void DistinctModuleNamesFromSeveralPackagesCoexist() + { + var first = BinaryPackage("FirstPack", "{ \"module\": \"FirstModule\", \"artifacts\": [] }"); + var second = BinaryPackage("SecondPack", "{ \"module\": \"SecondModule\", \"artifacts\": [] }"); + + var roots = PackageModuleBinder.Bind(WorkDirectory.Combine("Packages"), new[] { first, second }); + + Assert.That(roots, Has.Count.EqualTo(2)); + } + [Test] public void AnOverlayIsCopiedIntoThePackageItDescribes() { diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs b/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs index ee037df..3b51b37 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs @@ -170,6 +170,21 @@ public void AnOmittedTripletPinsTheSameAsTheExplicitHostTriplet() Assert.That(manifest.Dependencies["implicit"].PinKey("fmt"), Does.Contain(host)); } + /// + /// The pin ends up in the lock and in conflict messages, so it should read as the thing it + /// identifies - port and triplet, with no empty field standing in for a version rbt does not + /// support pinning (which vcpkg checkout you get is fixed by the pinned vcpkg tag instead). + /// + [Test] + public void TheVcpkgPinReadsAsPortAndTriplet() + { + var manifest = PackageManifest.Parse( + "{ \"dependencies\": { \"fmt\": { \"vcpkg\": \"fmt\", \"triplet\": \"x64-windows\" } } }", + "test".ToNPath()); + + Assert.That(manifest.Dependencies["fmt"].PinKey("fmt"), Is.EqualTo("vcpkg:fmt:x64-windows")); + } + [Test] public void TheTripletIsPartOfThePin() { From aa1843b3818593d30809947a4eb2e18fb6ad6392 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 14:50:01 +0000 Subject: [PATCH 08/10] fix(package): close git option injection and path escapes from manifests Four more from review, all variants of letting a manifest reach further than it should. A git URL or revision out of a manifest becomes an argv element. ArgumentList already rules out shell injection, but git itself still reads a leading '-' as an option, so a value like "--upload-pack=..." would have turned into a git flag. Both are now validated where the rest of the manifest is validated, once, rather than at each site that shells out. Rejecting the shape is portable; --end-of-options would have added a git 2.24 floor for the same effect. git clone also gets an explicit "--" so it does not depend on that check staying in place. A binary package's relative include and library paths could climb out of the package with "../..", putting arbitrary directories of the consuming machine on the include or library search path. Relative entries must now resolve inside the package. Absolute entries stay allowed on purpose - that is what the vcpkg bridge emits, since a vcpkg installed tree lives outside Packages/ by design. ProcessRunner silently ignored a working directory that did not exist, which meant the tool ran in rbt's own working directory instead: a git command against the wrong repository, failing in a way that gives no hint why. A directory the caller named now has to be there. ReBuildTool.Service gains InternalsVisibleTo for the test project, matching what ReBuildTool.CppCompiler already does, so the runner can be covered directly. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019nEaEVhDjn2dQC19BE9iaM --- .../Package/PackageArtifactSelector.cs | 22 ++++- .../Fetchers/GitPackageFetcher.cs | 5 +- .../PackageService/PackageManifest.cs | 43 ++++++++- .../PackageService/ProcessRunner.cs | 10 ++- .../ReBuildTool.Service.csproj | 5 ++ .../TestPackageBinaryModule.cs | 40 +++++++++ .../ReBuildTool.Test/TestPackageManifest.cs | 38 ++++++++ .../TestPackageProcessRunner.cs | 87 +++++++++++++++++++ 8 files changed, 243 insertions(+), 7 deletions(-) create mode 100644 ReBuildTool/ReBuildTool.Test/TestPackageProcessRunner.cs diff --git a/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageArtifactSelector.cs b/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageArtifactSelector.cs index 96a9797..90d6065 100644 --- a/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageArtifactSelector.cs +++ b/ReBuildTool/ReBuildTool.CppCompiler/Package/PackageArtifactSelector.cs @@ -87,8 +87,28 @@ private static bool Matches(string? declared, string actual) || string.Equals(declared, actual, StringComparison.OrdinalIgnoreCase); } + /// + /// Turns a manifest path into one the toolchain can use. + /// + /// A relative entry is the package describing its own layout, so it has to stay inside the + /// package: "../.." would otherwise let a package put arbitrary directories of the consuming + /// machine on the include or library search path. Absolute entries are left alone - that is + /// what the vcpkg bridge emits, since a vcpkg installed tree lives outside Packages/ by design. + /// private static string Resolve(NPath packageRoot, string path) { - return System.IO.Path.IsPathRooted(path) ? path : packageRoot.Combine(path).ToString(); + if (System.IO.Path.IsPathRooted(path)) + { + return path; + } + + var resolved = packageRoot.Combine(path); + if (!resolved.IsChildOf(packageRoot)) + { + throw new PackageException( + $"package at \"{packageRoot}\" declares the path \"{path}\", which resolves to " + + $"\"{resolved}\" - outside the package. Relative paths must stay within it."); + } + return resolved.ToString(); } } diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs index 6023af5..46c939a 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs @@ -35,7 +35,10 @@ public FetchedPackage Fetch(FetchRequest request) destination.EnsureParentDirectoryExists(); ProcessRunner.RunOrThrow( "git", - new[] { "clone", "--recurse-submodules", url, destination.ToString() }, + // "--" so the URL cannot be read as an option. The manifest validation already + // rejects a leading '-', but the marker costs nothing and does not rely on that + // check staying in place. + new[] { "clone", "--recurse-submodules", "--", url, destination.ToString() }, null, $"cloning package \"{request.Name}\""); } diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs index 6700009..2df2e0f 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifest.cs @@ -94,11 +94,18 @@ public PackageSourceKind ResolveKind(string packageName) $"set exactly one of \"git\", \"url\", \"path\" or \"vcpkg\"."); } - if (kinds[0] == PackageSourceKind.Git && Commit == null && Tag == null && Branch == null) + if (kinds[0] == PackageSourceKind.Git) { - throw new PackageException( - $"package \"{packageName}\" pins no git revision: set \"commit\", \"tag\" or \"branch\". " + - $"rbt resolves exact pins only - it never picks a version for you."); + if (Commit == null && Tag == null && Branch == null) + { + throw new PackageException( + $"package \"{packageName}\" pins no git revision: set \"commit\", \"tag\" or \"branch\". " + + $"rbt resolves exact pins only - it never picks a version for you."); + } + // Both reach git as argv elements, so both are checked here, once, rather than at each + // call site that shells out. + PackageNames.ValidateGitArgument(Git!, packageName, "url"); + PackageNames.ValidateGitArgument(GitRevision!, packageName, "revision"); } return kinds[0]; @@ -272,6 +279,34 @@ public static string ValidatePackageName(string name) return name; } + /// + /// Validates a manifest string that is handed to git as an argv element. + /// + /// ArgumentList already rules out shell injection, but git itself still parses a leading + /// '-' as an option - so a URL or revision out of a manifest could turn into a git flag + /// (--upload-pack=... and friends). Rejecting the shape outright is portable, unlike + /// --end-of-options, which needs git 2.24. + /// + public static string ValidateGitArgument(string value, string packageName, string what) + { + if (string.IsNullOrWhiteSpace(value)) + { + throw new PackageException($"package \"{packageName}\" declares an empty git {what}."); + } + if (value.StartsWith("-")) + { + throw new PackageException( + $"package \"{packageName}\" declares the git {what} \"{value}\", which starts with '-'. " + + $"git would read it as an option rather than a {what}."); + } + if (value.Any(character => char.IsControl(character))) + { + throw new PackageException( + $"package \"{packageName}\" declares a git {what} containing a control character."); + } + return value; + } + /// /// Validates a name that will be emitted into generated C# source. Beyond path safety, a name /// carrying punctuation could close the class declaration and append arbitrary code to the diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs b/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs index 97b95e5..a5589c1 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs @@ -39,8 +39,16 @@ public static ProcessResult Run(string program, IEnumerable arguments, N { startInfo.ArgumentList.Add(argument); } - if (workingDirectory != null && workingDirectory.DirectoryExists()) + if (workingDirectory != null) { + // Skipping a missing directory would silently run the tool in rbt's own working + // directory instead - a git command against the wrong repository, and a failure that + // gives no hint why. If a caller named a directory, it has to be there. + if (!workingDirectory.DirectoryExists()) + { + throw new PackageException( + $"cannot run \"{program}\": its working directory \"{workingDirectory}\" does not exist."); + } startInfo.WorkingDirectory = workingDirectory.ToString(); } diff --git a/ReBuildTool/ReBuildTool.Service/ReBuildTool.Service.csproj b/ReBuildTool/ReBuildTool.Service/ReBuildTool.Service.csproj index 4743bd5..7ce8c7f 100644 --- a/ReBuildTool/ReBuildTool.Service/ReBuildTool.Service.csproj +++ b/ReBuildTool/ReBuildTool.Service/ReBuildTool.Service.csproj @@ -14,4 +14,9 @@ + + + + + diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs b/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs index 23578d6..d24e3db 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageBinaryModule.cs @@ -205,6 +205,46 @@ public void ArtifactsAreSelectedByPlatformArchAndConfig() Assert.That(module.PublicLibraryDirectories.Single(), Is.EqualTo(root.Combine("lib").ToString())); } + /// + /// A relative path is the package describing its own layout. Letting it climb out would put + /// arbitrary directories of the consuming machine on the include or library search path. + /// + [TestCase("\\\"includes\\\": [\\\"../../elsewhere\\\"], \\\"artifacts\\\": []")] + [TestCase("\\\"artifacts\\\": [ { \\\"libraryDirectories\\\": [\\\"../../elsewhere\\\"] } ]")] + public void ARelativePathEscapingThePackageIsRejected(string binaryBody) + { + var context = BuildContext(); + var root = WorkDirectory.Combine("Pack").EnsureDirectoryExists(); + PackageManifest.PathIn(root).WriteAllText( + "{ \"name\": \"Pack\", \"binary\": { " + binaryBody.Replace("\\\"", "\"") + " } }"); + + var module = new SyntheticModule { ModuleDirectoryForTest = root.ToString() }; + var exception = Assert.Throws( + () => PackageArtifactSelector.Apply(module, context, PackageManifest.PathIn(root).ToString())); + + Assert.That(exception!.Message, Does.Contain("outside the package")); + } + + /// + /// Absolute entries stay allowed: that is exactly what the vcpkg bridge emits, because a vcpkg + /// installed tree lives outside Packages/ by design. + /// + [Test] + public void AnAbsolutePathIsPassedThrough() + { + var context = BuildContext(); + var elsewhere = WorkDirectory.Combine("vcpkg-ish").EnsureDirectoryExists(); + var root = WorkDirectory.Combine("Pack").EnsureDirectoryExists(); + PackageManifest.PathIn(root).WriteAllText( + "{ \"name\": \"Pack\", \"binary\": { \"includes\": [\"" + + elsewhere.ToString().Replace("\\", "\\\\") + "\"], \"artifacts\": [] } }"); + + var module = new SyntheticModule { ModuleDirectoryForTest = root.ToString() }; + PackageArtifactSelector.Apply(module, context, PackageManifest.PathIn(root).ToString()); + + Assert.That(module.PublicIncludePaths.Single(), Is.EqualTo(elsewhere.ToString())); + } + [Test] public void AnArtifactWithoutSelectorsMatchesEveryPlatform() { diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageManifest.cs b/ReBuildTool/ReBuildTool.Test/TestPackageManifest.cs index d1a82e5..c309e67 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageManifest.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageManifest.cs @@ -63,6 +63,44 @@ public void AGitDependencyWithoutARevisionIsRejected() Assert.That(exception.Message, Does.Contain("tag")); } + /// + /// ArgumentList rules out shell injection, but git still reads a leading '-' as an option, so a + /// manifest string could become a git flag. Rejected by shape, which works on any git version - + /// unlike --end-of-options, which needs 2.24. + /// + [TestCase("--upload-pack=touch /tmp/pwned")] + [TestCase("-c")] + public void AGitUrlThatGitWouldReadAsAnOptionIsRejected(string url) + { + var dependency = DependencyFrom( + $"{{ \"git\": \"{url}\", \"tag\": \"v1\" }}"); + + var exception = Assert.Throws(() => dependency.ResolveKind("Some")); + + Assert.That(exception!.Message, Does.Contain("option")); + } + + [TestCase("--upload-pack=x")] + [TestCase("-c")] + public void AGitRevisionThatGitWouldReadAsAnOptionIsRejected(string revision) + { + var dependency = DependencyFrom( + $"{{ \"git\": \"https://x/y.git\", \"commit\": \"{revision}\" }}"); + + var exception = Assert.Throws(() => dependency.ResolveKind("Some")); + + Assert.That(exception!.Message, Does.Contain("option")); + } + + [Test] + public void AnOrdinaryGitUrlAndRevisionArePreserved() + { + var dependency = DependencyFrom( + "{ \"git\": \"https://github.com/x/y.git\", \"tag\": \"v1.2.0-rc.1\" }"); + + Assert.DoesNotThrow(() => dependency.ResolveKind("Some")); + } + [Test] public void GitRevisionPrefersTheMostSpecificPin() { diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageProcessRunner.cs b/ReBuildTool/ReBuildTool.Test/TestPackageProcessRunner.cs new file mode 100644 index 0000000..0d5861c --- /dev/null +++ b/ReBuildTool/ReBuildTool.Test/TestPackageProcessRunner.cs @@ -0,0 +1,87 @@ +using NiceIO; +using ReBuildTool.Service.PackageService; + +namespace ReBuildTool.Test; + +/// +/// The external-tool runner the package fetchers shell out through. +/// +[TestFixture] +public class TestPackageProcessRunner +{ + private NPath WorkDirectory = null!; + + [SetUp] + public void SetUp() + { + WorkDirectory = Path.Combine(Path.GetTempPath(), $"rbt-runner-{Guid.NewGuid():N}") + .ToNPath() + .EnsureDirectoryExists(); + } + + [TearDown] + public void TearDown() + { + WorkDirectory.DeleteIfExists(DeleteMode.Normal); + } + + /// + /// Silently skipping a missing working directory would run the tool in rbt's own working + /// directory instead - a git command against the wrong repository, and a failure with no hint + /// as to why. A directory the caller named has to be there. + /// + [Test] + public void AMissingWorkingDirectoryIsAnError() + { + var missing = WorkDirectory.Combine("not-here"); + + var exception = Assert.Throws( + () => ProcessRunner.Run("git", new[] { "--version" }, missing)); + + Assert.That(exception!.Message, Does.Contain("not-here")); + Assert.That(exception.Message, Does.Contain("does not exist")); + } + + [Test] + public void ANullWorkingDirectoryIsFine() + { + // Cloning happens before the destination exists, so "no directory" stays legal. + var result = ProcessRunner.Run("git", new[] { "--version" }); + + Assert.That(result.IsSuccess, Is.True); + Assert.That(result.StdOut, Does.Contain("git version")); + } + + [Test] + public void StdOutIsCapturedInFull() + { + var result = ProcessRunner.Run("git", new[] { "--version" }, WorkDirectory); + + Assert.That(result.IsSuccess, Is.True); + Assert.That(result.StdOut.Trim(), Is.Not.Empty); + } + + /// A failure has to carry the tool's own diagnostics, or RunOrThrow reports nothing useful. + [Test] + public void AFailureCarriesTheToolsMessage() + { + var exception = Assert.Throws(() => ProcessRunner.RunOrThrow( + "git", + new[] { "rev-parse", "--verify", "definitely-not-a-ref" }, + WorkDirectory, + "resolving a revision")); + + Assert.That(exception!.Message, Does.Contain("resolving a revision")); + // Non-empty tail: the exact wording is git's, but something has to come back. + Assert.That(exception.Message.Length, Is.GreaterThan("resolving a revision".Length + 20)); + } + + [Test] + public void AMissingProgramIsReportedByName() + { + var exception = Assert.Throws( + () => ProcessRunner.Run("rbt-no-such-tool", Array.Empty())); + + Assert.That(exception!.Message, Does.Contain("rbt-no-such-tool")); + } +} From 97f12206698effe23e175550f73cdeb5c94e67ee Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 15:01:24 +0000 Subject: [PATCH 09/10] fix(package): re-resolve when the pin changes; host-correct vcpkg triplet A changed pin was silently ignored. The git fetcher reuses the commit the lock recorded rather than asking the remote again - that is what keeps an ordinary build reproducible and offline - but the lock entry was handed over without checking it came from the pin being resolved. Bumping a dependency's tag from v1.0 to v2.0 therefore resolved to the commit v1.0 pointed at, which still exists in the clone, and the build stayed on the old version while the manifest said otherwise. The resolver now only passes an entry whose pin matches. DefaultTriplet inferred the architecture from Is64BitOperatingSystem, which only separates 32- from 64-bit, so every arm64 host was handed an x64 triplet - and rbt targets Apple Silicon and arm64 Linux (Vendor/ninja ships a linux-aarch64 binary and CI runs a macOS arm64 leg). It now reads RuntimeInformation .OSArchitecture and throws for an architecture it has no mapping for, rather than installing binaries for the wrong machine and failing somewhere far from the cause. Its test asserted the old x86/x64-only behaviour and now checks the triplet against the actual host. --PackageAdd's grammar text claimed the '#' qualifier is required everywhere and that git pins are tag-only. Only git needs one, having no default revision to fall back on; a url without a sha256 is simply unverified and a vcpkg port without a triplet uses the host's. Docstring and error message now say so. ProcessRunner's WaitForExit gets a comment recording why the parameterless overload is the correct one - it is the version that waits for the redirected streams to drain, which is what keeps captured output complete. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019nEaEVhDjn2dQC19BE9iaM --- .../Fetchers/VcpkgPackageFetcher.cs | 18 ++++++++- .../PackageService/PackageManifestEditor.cs | 14 +++++-- .../PackageService/PackageResolver.cs | 18 ++++++++- .../PackageService/ProcessRunner.cs | 4 ++ .../ReBuildTool.Test/TestPackageRestore.cs | 39 +++++++++++++++++++ .../ReBuildTool.Test/TestPackageVcpkg.cs | 20 +++++++++- 6 files changed, 106 insertions(+), 7 deletions(-) diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs index b6eb999..f0df247 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs @@ -1,3 +1,4 @@ +using System.Runtime.InteropServices; using NiceIO; using Newtonsoft.Json; using ReBuildTool.Service.Global; @@ -208,7 +209,22 @@ private static NPath VcpkgExecutable() /// public static string DefaultTriplet() { - var architecture = Environment.Is64BitOperatingSystem ? "x64" : "x86"; + // Not Is64BitOperatingSystem: that only distinguishes 32- from 64-bit, so every arm64 host + // would silently be handed an x64 triplet - and rbt targets Apple Silicon and arm64 Linux + // (Vendor/ninja ships a linux-aarch64 binary, and CI runs a macOS arm64 leg). + var architecture = RuntimeInformation.OSArchitecture switch + { + Architecture.X64 => "x64", + Architecture.X86 => "x86", + Architecture.Arm64 => "arm64", + Architecture.Arm => "arm", + // Guessing here would install binaries for the wrong machine, which fails far away from + // the cause. Better to say so and let the user name the triplet. + var other => throw new PackageException( + $"no default vcpkg triplet for host architecture {other}. " + + $"Set \"triplet\" explicitly on the vcpkg dependency.") + }; + if (PlatformHelper.IsWindows()) { return $"{architecture}-windows"; diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifestEditor.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifestEditor.cs index 685248a..c2af2a5 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifestEditor.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageManifestEditor.cs @@ -15,9 +15,14 @@ namespace ReBuildTool.Service.PackageService; public static class PackageManifestEditor { /// - /// Parses the compact spec accepted by --PackageAdd: - /// git:<url>#<tag-or-commit>, path:<dir>, - /// url:<href>#<sha256> or vcpkg:<port>#<triplet>. + /// Parses the compact spec accepted by --PackageAdd. The qualifier after '#' is required + /// only for git, which has no default revision to fall back on: + /// + /// git:<url>#<tag-or-commit> + /// path:<dir> + /// url:<href>[#<sha256>] - without a checksum the archive is not verified + /// vcpkg:<port>[#<triplet>] - the triplet defaults to the host's + /// /// public static PackageDependency ParseSpec(string spec) { @@ -26,7 +31,8 @@ public static PackageDependency ParseSpec(string spec) { throw new PackageException( $"cannot read package spec \"{spec}\": expected one of " + - $"git:#, path:, url:#, vcpkg:#."); + $"git:#, path:, url:[#], " + + $"vcpkg:[#]."); } var kind = spec.Substring(0, separator).ToLowerInvariant(); diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs b/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs index ac02ddd..4452f6a 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/PackageResolver.cs @@ -122,7 +122,7 @@ private void ResolveOne(string name, PackageDependency declared, NPath declaring declaringDirectory, PackagesRoot, Options, - ExistingLock?.Find(name)); + LockedFor(name, pinKey)); var fetched = fetcher.Fetch(request); var manifest = PackageManifest.ReadFrom(fetched.Root); @@ -169,6 +169,22 @@ private void ResolveOne(string name, PackageDependency declared, NPath declaring Log.Info($"[package] {name} -> {Resolved[name].Locked.Resolved}"); } + /// + /// The lock entry for a package, but only when it was produced from the pin currently being + /// resolved. + /// + /// A fetcher treats the entry as "what this pin resolved to last time" and may reuse it instead + /// of consulting the remote - that is what keeps an ordinary build reproducible and offline. + /// Handing over an entry from a different pin turns that shortcut into a trap: bumping a + /// dependency's tag in the manifest would resolve to the commit the *old* tag pointed at, and + /// the build would silently stay on the previous version. + /// + private LockedPackage? LockedFor(string name, string pinKey) + { + var locked = ExistingLock?.Find(name); + return locked?.Pin == pinKey ? locked : null; + } + /// /// Resolves a dependency's overlay against the manifest that declared it - the rule file /// belongs to whoever is consuming the package, not to the package itself. diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs b/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs index a5589c1..2f698d6 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/ProcessRunner.cs @@ -83,6 +83,10 @@ public static ProcessResult Run(string program, IEnumerable arguments, N process.BeginOutputReadLine(); process.BeginErrorReadLine(); + // The parameterless WaitForExit is the overload that also waits for the redirected streams + // to reach EOF and the async handlers above to drain - that is precisely what separates it + // from WaitForExit(int), which returns as soon as the process is gone and can leave the + // last lines uncollected. Do not "optimise" this into the timeout overload. process.WaitForExit(); return new ProcessResult diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs b/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs index 09ecc16..91a36dd 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs @@ -125,6 +125,45 @@ public void ASecondRestoreSucceedsOffline() new PackageRestoreService().Restore(project, new PackageRestoreOptions { Offline = true })); } + /// + /// Bumping a dependency's tag has to actually move the checkout. + /// + /// The fetcher reuses the commit the lock recorded rather than asking the remote again - that + /// is what keeps an ordinary build reproducible and offline. Handing it a lock entry from a + /// different pin turns that shortcut into a trap: the old tag's commit still resolves locally, + /// so the build silently stays on the previous version while the manifest says otherwise. + /// + [Test] + public void ChangingTheTagReResolvesInsteadOfReusingTheLock() + { + var repository = CreateLibraryRepository("GreeterLib"); + var project = CreateProject( + "{ \"dependencies\": { \"GreeterLib\": { " + + $"\"git\": \"{repository.ToString(SlashMode.Forward)}\", \"tag\": \"v1.0\" }} }} }}"); + new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + var firstSha = PackageLockFile.ReadFrom(project)!.Find("GreeterLib")!.Resolved; + + // A second release upstream, tagged v2.0. + repository.Combine("GreeterLib.module.cs").WriteAllText( + $"using ReBuildTool.ToolChain;{Environment.NewLine}" + + $"public class GreeterLib : CppModuleRule {{ /* v2 */ }}{Environment.NewLine}"); + Git(repository, "add", "."); + Git(repository, "commit", "-m", "second"); + Git(repository, "tag", "v2.0"); + var secondSha = Git(repository, "rev-parse", "v2.0^{commit}"); + Assert.That(secondSha, Is.Not.EqualTo(firstSha)); + + PackageManifest.PathIn(project).WriteAllText( + "{ \"dependencies\": { \"GreeterLib\": { " + + $"\"git\": \"{repository.ToString(SlashMode.Forward)}\", \"tag\": \"v2.0\" }} }} }}"); + new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + + Assert.That(PackageLockFile.ReadFrom(project)!.Find("GreeterLib")!.Resolved, + Is.EqualTo(secondSha), "the new tag should have been resolved, not the locked commit"); + Assert.That(project.Combine("Packages", "GreeterLib", "GreeterLib.module.cs").ReadAllText(), + Does.Contain("v2"), "the working tree should have moved to the new commit"); + } + [Test] public void AnUnfetchedPackageCannotBeRestoredOffline() { diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs b/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs index 3b51b37..6bb5f39 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs @@ -141,12 +141,30 @@ public void NonLibraryFilesAreNotLinked() Is.EqualTo(new[] { "libthing.a" })); } + /// + /// The architecture has to come from the host's real architecture, not merely its bitness: + /// rbt targets Apple Silicon and arm64 Linux, and an arm64 host handed an x64 triplet would + /// install binaries for the wrong machine. + /// [Test] public void TheHostTripletIsUsedWhenNoneIsDeclared() { var triplet = VcpkgPackageFetcher.DefaultTriplet(); - Assert.That(triplet, Does.Match(@"^(x64|x86)-(windows|osx|linux)$")); + Assert.That(triplet, Does.Match(@"^(x64|x86|arm64|arm)-(windows|osx|linux)$")); + + var expectedArchitecture = System.Runtime.InteropServices.RuntimeInformation.OSArchitecture switch + { + System.Runtime.InteropServices.Architecture.X64 => "x64", + System.Runtime.InteropServices.Architecture.X86 => "x86", + System.Runtime.InteropServices.Architecture.Arm64 => "arm64", + System.Runtime.InteropServices.Architecture.Arm => "arm", + _ => null + }; + if (expectedArchitecture != null) + { + Assert.That(triplet, Does.StartWith($"{expectedArchitecture}-")); + } } /// From b66c3f88202f7465819fbbabc559050ce09d3ac3 Mon Sep 17 00:00:00 2001 From: ResetOTER Date: Wed, 29 Jul 2026 00:40:54 +0800 Subject: [PATCH 10/10] fix(package): validate git origins and vcpkg ownership --- .../Fetchers/GitPackageFetcher.cs | 44 +++++++++++++++ .../Fetchers/VcpkgPackageFetcher.cs | 54 ++++++++++++++++--- .../ReBuildTool.Test/TestPackageRestore.cs | 22 ++++++++ .../ReBuildTool.Test/TestPackageVcpkg.cs | 38 +++++++++++++ 4 files changed, 150 insertions(+), 8 deletions(-) diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs index 46c939a..ad80639 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/GitPackageFetcher.cs @@ -22,6 +22,28 @@ public FetchedPackage Fetch(FetchRequest request) var destination = request.DefaultDestination; var isClone = destination.Combine(".git").DirectoryExists(); + if (isClone) + { + var existingUrl = ProcessRunner.RunOrThrow( + "git", + new[] { "remote", "get-url", "origin" }, + destination, + $"reading the origin of package \"{request.Name}\""); + if (!string.Equals(existingUrl, url, StringComparison.Ordinal)) + { + // Git objects and refs survive a remote URL change. Reusing this clone could + // therefore resolve a tag or commit from the old repository while the lock claims + // it came from the new one. A fresh clone is the only reliable way to keep the + // object database tied to the declared origin. + RequireNetwork( + request, + $"package \"{request.Name}\" changed its git origin from \"{existingUrl}\" to \"{url}\""); + Log.Info($"[package] origin changed for {request.Name}; cloning it again from {url}"); + DeleteClone(destination); + isClone = false; + } + } + if (!isClone) { // A leftover directory that is not a clone (an interrupted fetch, or a rename) would @@ -147,4 +169,26 @@ private static void RequireNetwork(FetchRequest request, string why) $"--Offline was requested but {why}. Run a restore without --Offline first."); } } + + private static void DeleteClone(NPath destination) + { + // Git object files can be read-only on Windows. Directory.Delete reports those as + // UnauthorizedAccessException, so clear only that bit inside the exact package directory + // before replacing the clone. + if (OperatingSystem.IsWindows()) + { + foreach (var file in Directory.EnumerateFiles( + destination.ToString(), + "*", + SearchOption.AllDirectories)) + { + var attributes = File.GetAttributes(file); + if ((attributes & FileAttributes.ReadOnly) != 0) + { + File.SetAttributes(file, attributes & ~FileAttributes.ReadOnly); + } + } + } + destination.DeleteIfExists(DeleteMode.Normal); + } } diff --git a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs index f0df247..50a3a69 100644 --- a/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs +++ b/ReBuildTool/ReBuildTool.Service/PackageService/Fetchers/VcpkgPackageFetcher.cs @@ -40,8 +40,8 @@ public FetchedPackage Fetch(FetchRequest request) var installed = VcpkgRoot.Combine("installed", triplet); var destination = request.DefaultDestination; - var alreadyInstalled = installed.DirectoryExists() - && VcpkgRoot.Combine("installed", "vcpkg", "info").DirectoryExists(); + var infoRoot = VcpkgRoot.Combine("installed", "vcpkg", "info"); + var alreadyInstalled = IsPortInstalled(installed, infoRoot, port, triplet); if (!alreadyInstalled || request.Options.Force) { @@ -97,8 +97,16 @@ public static string DescribeInstalledTree(string packageName, string port, NPat // vcpkg keeps the debug build in a parallel debug/ prefix. Mapping it to rbt's Debug // configuration is the whole reason this is not a single artifact. - var release = LibrariesIn(installed.Combine("lib")); - var debug = LibrariesIn(installed.Combine("debug", "lib")); + var infoRoot = installed.Parent.Combine("vcpkg", "info"); + var ownedFiles = PortInfoFiles(infoRoot, port, installed.FileName) + .SelectMany(file => file.ReadAllLines()) + .Select(path => path.Replace('\\', '/')) + .ToList(); + var release = LibrariesIn(installed.Combine("lib"), ownedFiles, $"{installed.FileName}/lib/"); + var debug = LibrariesIn( + installed.Combine("debug", "lib"), + ownedFiles, + $"{installed.FileName}/debug/lib/"); if (debug.Count > 0) { @@ -144,16 +152,46 @@ public static string DescribeInstalledTree(string packageName, string port, NPat return JsonConvert.SerializeObject(manifest, Formatting.Indented) + Environment.NewLine; } - private static List LibrariesIn(NPath directory) + internal static bool IsPortInstalled(NPath installed, NPath infoRoot, string port, string triplet) + { + return installed.DirectoryExists() && PortInfoFiles(infoRoot, port, triplet).Any(); + } + + private static IEnumerable PortInfoFiles(NPath info, string port, string triplet) + { + if (!info.DirectoryExists()) + { + return Array.Empty(); + } + + var suffix = $"_{triplet}.list"; + return info.Files("*.list") + .Where(file => file.FileName.StartsWith($"{port}_", StringComparison.OrdinalIgnoreCase) + && file.FileName.EndsWith(suffix, StringComparison.OrdinalIgnoreCase)) + .ToList(); + } + + private static List LibrariesIn( + NPath directory, + IEnumerable ownedFiles, + string relativePrefix) { if (!directory.DirectoryExists()) { return new List(); } - return directory.Files() - .Where(file => file.ExtensionWithDot is ".lib" or ".a") - .Select(file => file.FileName) + + // The triplet's lib directories are shared by every installed port. The .list files under + // installed/vcpkg/info are vcpkg's ownership records; only entries owned by this port may + // become link inputs for the synthesized module. + return ownedFiles + .Where(path => path.StartsWith(relativePrefix, StringComparison.OrdinalIgnoreCase)) + .Select(path => path.Substring(relativePrefix.Length)) + .Where(path => !path.Contains('/')) + .Where(path => path.EndsWith(".lib", StringComparison.OrdinalIgnoreCase) + || path.EndsWith(".a", StringComparison.OrdinalIgnoreCase)) .OrderBy(name => name, StringComparer.Ordinal) + .Distinct(StringComparer.OrdinalIgnoreCase) .ToList(); } diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs b/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs index 91a36dd..630c278 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageRestore.cs @@ -164,6 +164,28 @@ public void ChangingTheTagReResolvesInsteadOfReusingTheLock() Does.Contain("v2"), "the working tree should have moved to the new commit"); } + [Test] + public void ChangingTheGitOriginReclonesInsteadOfUsingOldObjects() + { + var first = CreateLibraryRepository("First"); + var second = CreateLibraryRepository("Second"); + var project = CreateProject( + "{ \"dependencies\": { \"SharedName\": { " + + $"\"git\": \"{first.ToString(SlashMode.Forward)}\", \"tag\": \"v1.0\" }} }} }}"); + new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + + PackageManifest.PathIn(project).WriteAllText( + "{ \"dependencies\": { \"SharedName\": { " + + $"\"git\": \"{second.ToString(SlashMode.Forward)}\", \"tag\": \"v1.0\" }} }} }}"); + new PackageRestoreService().Restore(project, new PackageRestoreOptions()); + + var package = project.Combine("Packages", "SharedName"); + Assert.That(package.Combine("Second.module.cs").FileExists(), Is.True); + Assert.That(package.Combine("First.module.cs").FileExists(), Is.False); + Assert.That(Git(package, "remote", "get-url", "origin"), + Is.EqualTo(second.ToString(SlashMode.Forward))); + } + [Test] public void AnUnfetchedPackageCannotBeRestoredOffline() { diff --git a/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs b/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs index 6bb5f39..32bfd0c 100644 --- a/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs +++ b/ReBuildTool/ReBuildTool.Test/TestPackageVcpkg.cs @@ -36,14 +36,21 @@ private NPath FakeInstalledTree(string[] releaseLibraries, string[] debugLibrari { var installed = WorkDirectory.Combine("installed", "x64-linux"); installed.Combine("include").EnsureDirectoryExists().Combine("thing.h").WriteAllText("#pragma once"); + var ownedFiles = new List { "x64-linux/include/thing.h" }; foreach (var library in releaseLibraries) { installed.Combine("lib").EnsureDirectoryExists().Combine(library).WriteAllText(""); + ownedFiles.Add($"x64-linux/lib/{library}"); } foreach (var library in debugLibraries) { installed.Combine("debug", "lib").EnsureDirectoryExists().Combine(library).WriteAllText(""); + ownedFiles.Add($"x64-linux/debug/lib/{library}"); } + WorkDirectory.Combine("installed", "vcpkg", "info") + .EnsureDirectoryExists() + .Combine("someport_1.0_x64-linux.list") + .WriteAllLines(ownedFiles.ToArray()); return installed; } @@ -141,6 +148,37 @@ public void NonLibraryFilesAreNotLinked() Is.EqualTo(new[] { "libthing.a" })); } + [Test] + public void LibrariesOwnedByOtherPortsAreNotLinked() + { + var installed = FakeInstalledTree(new[] { "libthing.a" }, Array.Empty()); + installed.Combine("lib", "libother.a").WriteAllText(""); + WorkDirectory.Combine("installed", "vcpkg", "info", "other_2.0_x64-linux.list") + .WriteAllText("x64-linux/lib/libother.a"); + + var manifest = Describe("Thing", installed); + + Assert.That( + manifest.Binary!.Artifacts.SelectMany(artifact => artifact.StaticLibraries).Distinct(), + Is.EqualTo(new[] { "libthing.a" })); + } + + [Test] + public void AnotherInstalledPortDoesNotSatisfyThisPort() + { + var installed = FakeInstalledTree(Array.Empty(), Array.Empty()); + var info = WorkDirectory.Combine("installed", "vcpkg", "info"); + info.Combine("someport_1.0_x64-linux.list").Delete(); + info.Combine("other_2.0_x64-linux.list").WriteAllText("x64-linux/include/other.h"); + + Assert.That( + VcpkgPackageFetcher.IsPortInstalled(installed, info, "someport", "x64-linux"), + Is.False); + Assert.That( + VcpkgPackageFetcher.IsPortInstalled(installed, info, "other", "x64-linux"), + Is.True); + } + /// /// The architecture has to come from the host's real architecture, not merely its bitness: /// rbt targets Apple Silicon and arm64 Linux, and an arm64 host handed an x64 triplet would