Rework targets and props for SDK#3
Merged
Merged
Conversation
- Sdk.props: DebugType=full for Debug, set BEFORE the Microsoft.NET.Sdk import so its '==empty -> portable' default is pre-empted. Under VS's .NET-Framework csc this yields a Windows/full PDB and source breakpoints bind; a portable PDB makes VS bind at the method entry only (one startup hit, never on the line). Release/CLI fall through to portable. - Sdk.targets: remove the LaunchProfiles capability so the C# project system's launcher doesn't own F5 (the NanoDebugger flavor + rule route F5 to the device); surface the NanoDebugger property-page rule via PropertyPageSchema. - Add Sdk/Rules/NanoDebugger.xaml and pack it. Proven by the SDK-style POC (deploy + F5 + source breakpoints on a real ESP32_S3_OCTAL) — nanoframework/Home#1784. Demo: https://youtu.be/9qvXsgXCrjM Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Sdk.props: bump the auto-injected nanoFramework.Tools.MetadataProcessor.MsBuildTask
3.0.29 -> 4.0.0-preview.94 (4.x emits the v2 PE format NFMRK2; 3.0.x emits v1/NFMRK1).
- Sdk.targets: MDP task TFM for Core MSBuild net6.0 -> net8.0 (4.x ships net8.0 + net472).
Validated: SmokeTest built with VS MSBuild against the modified SDK emits a v2 PE (NFMRK2)
and a Windows/full PDB ('Microsoft C/C++ MSF') so source breakpoints bind.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… dir) Relocate the legacy .nfproj -> SDK-style .csproj migration tool into this repo, where it belongs alongside the SDK. Hardened for a safe single-repo fleet run: - Idempotent + reentrant: skips projects whose root already has an Sdk attribute; re-running is a no-op (never re-converts or empties an already-converted project). - Default output is .csproj (deletes the original .nfproj); namespace-agnostic parsing. - Emits the correct versionless <Project Sdk="nanoFramework.NET.Sdk"> reference. - Derives PackageReference id+version from the packages\<Id>.<Version>\ HintPath folder (alias table is fallback only); skips with a review note instead of emitting Version="". - Deletes a hand-written Properties/AssemblyInfo.cs (avoids duplicate-attribute errors). - Rewrites the .sln entry (project-type GUID + .nfproj->.csproj path), line-scoped so a mixed/partial repo and reruns stay correct. - Never clobbers an existing .bak backup. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A fully-converted tree has no .nfproj left; return 0 with a 'nothing to convert' message instead of erroring, so re-running the converter over a repo is a safe no-op. Verified at fleet scale: 143 .nfproj converted in one pass; re-run is a clean no-op; a mixed tree converts only the remaining .nfproj and leaves existing .csproj untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Avoid the large monolithic Sdk.props (~150 lines) and Sdk.targets (~478 lines) by carving them into focused modules, mirroring the POC layout. Behavior-preserving (relocate-only, no logic rewrites); verified. - Sdk.props (76 lines) — thin orchestrator: keeps the DebugType=full Debug pre-emption BEFORE the Microsoft.NET.Sdk props import (it must pre-empt the SDK's portable default), owns the import chain, sets _NfSdkDir/_NfSdkRoot, imports the TFM module. - Sdk.targets (117 lines) — thin orchestrator: keeps the IsCoreAssembly GenerateAssembly* suppressions before the SDK import, then imports the MDP + Capabilities modules. - nanoFramework.Tfm.props (72) — TFM identity (netnano1.0, .NETnanoFramework, monikers) + compiler/TFM-shaping props (NoStdLib, TargetingClr2Framework, AssetTargetFallback, etc.). - nanoFramework.Mdp.targets (412) — the Metadata Processor pipeline: auto-injected MDP PackageReference (IsImplicitlyDefined, 4.0.0-preview.94), _NfMdpTasksTFM (net8.0/net472), and all PE/pdbx/resource/clean targets. - nanoFramework.Capabilities.targets (39) — CPS capabilities (remove LaunchProfiles, add NanoCSharpProject) + the NanoDebugger.xaml PropertyPageSchema. - csproj packs the three new modules alongside Sdk.props/Sdk.targets/Rules. Verified: repacked 1.0.0; SmokeTest builds under VS MSBuild (PE=NFMRK2, PDB=Microsoft C/C++ MSF Windows/full) and under dotnet build (PE=NFMRK2). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make the converter pleasant and safe to drive interactively, and add glob scoping.
- Spectre.Console (0.57.0): title rule, status spinner, a color-coded summary table
(Project | Result | Packages | Notes), a grouped "manual review" panel, and a tally line.
Degrades gracefully when output is redirected / non-interactive.
- --glob <pattern>: filter .nfproj by path relative to the input dir (*, **, ? supported);
default = all recursively. Works for migrate and fleet.
- Test-first: --dry-run renders an exact preview (target .csproj, files to delete, .sln edits)
and writes nothing; a real interactive run confirms once ("Proceed with N conversions?")
unless --yes or non-interactive (CI proceeds automatically).
- Richer ConvertResult (status enum + packages/deletions/sln/error) feeds the preview; all
real-run side effects unchanged. Exit codes: 0 clean, 2 review-flagged, 1 error.
- nuget.config now includes nuget.org (Spectre.Console must restore once; offline thereafter).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The migration skill shipped as a binary .skill (a zip bundling a stale copy of the converter source). Replace it with a proper, installable text skill under skills/nanoframework-sdk-migration: - SKILL.md — frontmatter (name + trigger-rich description) and a concise, action-oriented body that drives the tool via `dotnet nano migrate` with the test-first workflow (dry-run a directory, review, scope with --glob, then run for real) — no bundled/stale source. - references/migration-rules.md, references/contributing-compliance.md — the conversion rules and contribution/PR conventions. Also delete the obsolete Python reference converter (tools/NanoMigrate/nano-migrate.py); the hardened C# tool is canonical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ts; NuGet-ready Split the single console project into a small solution that separates the conversion engine from presentation, with unit tests, all packable. - NanoMigrate.Core (net8.0 library, no console deps): IProjectConverter/ProjectConverter, ConversionOptions, ConvertResult/ConvertStatus (results as data), ProjectScanner, Glob, FleetService (git side-effects behind IGitRunner). Packs as nanoFramework.Migrate.Core. - NanoMigrate.Cli (net8.0, PackAsTool nano-migrate): Spectre.Console.Cli CommandApp with typed Migrate/Clone/Fleet commands (auto-generated help), Rendering/ holds all AnsiConsole output. Packs as the nanoFramework.Migrate tool. - NanoMigrate.Tests (xUnit): 28 tests over the engine — HintPath id/version, glob, idempotency, packages.config mapping, unresolved-ref review note, .sln rewrite, full convert. All pass. Behavior + exit-code contract preserved (logic moved, not rewritten). Spectre pinned to 0.55.0 (latest with a matching Spectre.Console.Cli). One real glob fix: `Beginner/**` now matches `Beginner` itself, per the documented intent + a test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Migrate can now be driven by solutions, and updates them:
- A .sln or .slnx can be the input: only the projects referenced by that solution are converted,
and the solution is retargeted (classic .sln: project-type GUID {11A8DD76..}->{9A19103F..} +
.nfproj->.csproj path; .slnx: Path .nfproj->.csproj).
- Directory input, no solution: discover all .sln/.slnx. None -> loose directory mode (unchanged).
One or more -> the user selects which solution(s) to migrate (Spectre MultiSelectionPrompt:
all/none/multiple); only the chosen solutions' projects convert and those solutions update.
- Glob input: find matching .nfproj, then discover the solutions that reference them by analysing
solution content; confirm; multi-select when several are affected; update only those solutions.
- --solution <path> forces a single target; --yes / non-interactive selects all affected and
proceeds.
Pure logic in Core (SolutionFile parser for both formats, SolutionScanner, SolutionRewriter,
SolutionDiscovery, MigrationPlanner) with 16 new unit tests (44 total, all green); prompts/rendering
in the Cli. Idempotent: re-running a converted solution is a no-op.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ement support Two converter improvements; the real Samples dry-run goes from 55 review-flags to 0. - packages.config is now the authoritative PackageReference source: when present, emit one PackageReference per <package> (id+version verbatim) and drop the legacy <Reference> assembly elements. This fixes the assembly-name vs package-id mismatch (e.g. assembly System.Net.Http is shipped by package nanoFramework.System.Net.Http.Server; mscorlib by nanoFramework.CoreLibrary). The <Reference>+HintPath path remains a fallback only when there is no packages.config. - Central Package Management: detect a Directory.Packages.props (ManagePackageVersionsCentrally) above the project; when active, emit versionless PackageReference and seed missing <PackageVersion> entries into the nearest central props (idempotent). Never crashes on already-central / versionless inputs. - Carry through nanoFramework unit-test markers (ProjectCapability TestContainer, IsTestProject, TestProjectType, RunSettingsFilePath) so migrated test projects stay test projects. Tests: +11 (55 total, all green). Dry-run over the real Samples repo: 152 convert, 0 flagged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ed OutputType) The converter only carried a fixed keep-list of properties and silently dropped the rest, so <OutputType>Exe</OutputType> was lost — every app project became a library and failed to build (CS8805 for top-level-statement apps). Switch to pass-through: keep every property except the project-system boilerplate (DropProps) and the TFM the converter emits itself. Adds an OutputType regression test (56 total, all green). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Spot-building migrated samples surfaced several conversion-caused build errors; fix them so the output compiles: - Default-globbed EmbeddedResource/Content/None: drop plain ones (SDK globs them), rewrite metadata-bearing ones from Include= to Update= (fixes NETSDK1022 duplicate items), keep Link/external as Include. Emit child metadata (Generator/LastGenOutput/...). - Over-globbing: emit <Compile Remove=...> for on-disk .cs not in the legacy explicit subset (fixes CS0101/CS0111 duplicate types). - Native-stub libraries: keep Properties/AssemblyInfo.cs + set GenerateAssemblyInfo=false when it declares [assembly: AssemblyNativeVersion] (the SDK never generates it). - Shared projects: carry through .projitems <Import> (real source); flag other unknown imports. Tests: +13 (69 total, all green). On a full migrated copy, the representative set across shapes (app, resx, library, unit-test, shared-project, interop) builds to NFMRK2. Remaining non-building projects are out-of-scope (desktop .NET FW projects, pre-existing package-version conflicts, a missing-dep sample, a bin\ HintPath sibling reference). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The legacy NFProjectSystem defined NANOFRAMEWORK_1_0; the SDK only defined NETNANO1_0, so existing source with #if NANOFRAMEWORK_1_0 compiled the wrong branch (e.g. pulled System.Linq). Define both. Verified: a sample using #if !NANOFRAMEWORK_1_0 now builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… wrapper)
Scaffold the single `dotnet nano <command>` CLI that ships with the SDK (PackAsTool,
ToolCommandName=nano, PackageId=nanoFramework.Tool), per docs nano-tool.md.
- Spectre.Console.Cli CommandApp host with examples + descriptions; exit-code mapping.
- Built-in `migrate` runs in-proc over the conversion engine. To stay DRY, the shared
MigrateCommand/MigrateSettings/renderer were factored out of NanoMigrate.Cli into a new
NanoMigrate.Cli.Commands library that both the standalone nano-migrate CLI and the umbrella
reference (one implementation). NanoMigrate.Core flows transitively.
- External tools: IExternalTool + ExternalToolResolver (resolution order bundled -> installed/PATH
-> user cache -> download, behind an injectable environment seam; download is a stubbed
interface). NanoffTool wraps nanoff; nano-tools.json (embedded) pins it. `flash` maps
--target/--port onto nanoff and errors cleanly when nanoff is absent.
- deploy/monitor/devices are discoverable placeholders ("use VS/VS Code for now").
- nanoFramework.Tool.slnx covers the tool, its tests, and the NanoMigrate projects it references;
the netstandard2.0 SDK solution is left untouched.
Verified: both solutions build; 69 NanoMigrate + 8 umbrella tests pass; `nano --help` lists all
commands; `nano migrate --help` shows the engine options; `nano flash` errors cleanly without
nanoff; `dotnet pack` produces the tool package (command `nano`); umbrella dry-run writes nothing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… command - Verify: after a real migration, build the affected solution(s)/project(s) via dotnet (SolutionBuilder behind IBuildRunner); --verify/--no-verify (on for real runs, off for dry-run). A failed build offers rollback. - Rollback: a git-independent journal (.nanomigrate/rollback-<id>/ + manifest.json) backs up every file to be modified/deleted before any write; on verify failure it prompts (interactive) or advises `rollback <path>` (non-interactive, never auto-reverts); an explicit `rollback` command reverts the last recorded migration. Restores .nfproj/packages.config/AssemblyInfo/.sln byte-equal and deletes the generated .csproj. - Clean: `clean <path>` removes *.nfproj.bak and .nanomigrate/ leftovers (confirm / --yes). - Logic in Core (RollbackJournal, SolutionBuilder, BackupCleaner, Verification, MigrationJournaling); commands in Cli.Commands (CleanCommand, RollbackCommand, MigrateRegistration) shared by the standalone nano-migrate CLI and the nano umbrella. clean/rollback are top-level commands (Spectre.Cli 0.55 can't host a default/branch command that also takes a positional). Tests: +22 (91 total) + 8 umbrella, all green; both solutions build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…Persistence Adopt the official .NET Foundation solution library (1.0.52) for .sln AND .slnx, so we no longer hand-maintain solution parsing/writing: - SolutionFile reads both formats via SolutionSerializers (stream-based), exposing the same surface (ProjectPaths, NanoProjects, Format). SolutionRewriter retargets by mutating the SolutionModel (path .nfproj->.csproj; SDK C# type GUID on classic .sln; cleared on .slnx) and re-serializing — the hand-rolled regex/line/text editors are deleted. Idempotent no-op returns original bytes. - Compatibility shim: the .slnx serializer rejects a typeless <Project> for the unknown .nfproj extension, so we inject the legacy nano type GUID and retry; VS-authored .slnx parse directly. Project files (.csproj/.nfproj) intentionally keep the controlled emit / XElement read: adopting Microsoft.Build in a self-contained net8.0 tool is fragile (MSBuildLocator finds no MSBuild against an SDK-only host) and shipping its runtime assemblies pulls a high-severity advisory + a heavy toolset. The csproj emitter we own is small and low-maintenance. Tests: +3 round-trip (parse -> retarget -> reparse sees .csproj, classic + slnx + VS-typed slnx); 94 total + 8 umbrella, all green; both solutions build; the packed nano tool bundles the library. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pure structural refactor (no behavior change): the 18 Core files are grouped into feature slices, each with its own sub-namespace: - Projects/ (ProjectConverter, ConversionOptions, ConvertResult/Status, IProjectConverter) - Solutions/ (SolutionFile, SolutionRewriter, SolutionScanner, SolutionDiscovery, MigrationPlan) - Backup/ (RollbackJournal, BackupCleaner, MigrationJournaling) - Verification/ (SolutionBuilder, Verification) - Fleet/ (FleetService, RepoReport) - Common/ (Glob, ProjectScanner) Consumers use the slice namespaces via global <Using> items (ImplicitUsings). Engine stays console-free. 94 + 8 tests pass; both solutions build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an opt-in migration report. After a migrate run (including --dry-run), --report <path> writes a report; the format is chosen by extension (.md/.markdown -> Markdown, .html/.htm -> HTML). - Core Reporting/ slice (pure, console-free): MigrationReport model (caller supplies the UTC timestamp), MarkdownReportWriter, HtmlReportWriter. Summary totals + a Project|Result|Packages|Notes table + manual-review + verify sections; HTML is self-contained (inline CSS, color-coded) with all text HTML-escaped. - Cli.Commands: MigrationReportBuilder maps the run's outcomes (incl. verify results) onto the model; --report option writes it and reports the path. Write failures degrade to a warning. No clean/auto-clean behavior added. Tests: +11 (105 total) + 8 umbrella, all green; both solutions build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ournal-internal backups The rollback journal is self-contained in .nanomigrate/rollback-<id>/ (it backs up every original it restores), and the converter's loose *.nfproj.bak is purely opt-in (suppressed by --no-backup). Fix: BackupCleaner no longer counts the journal's internal <seq>-*.nfproj.bak (inside .nanomigrate/) as a loose backup, so "loose backups" and "rollback folders" are disjoint. Clarified the ProjectConverter/MigrationJournaling docs. Result: `migrate --no-backup` (real run) writes 0 next-to-project .bak, keeps a working journal, and `rollback` restores byte-equal from .nanomigrate/. Tests +3 (108 total).
Relocate the tool so all CLI projects live under tools/: - src/nanoFramework.Tool -> tools/nano/nanoFramework.Tool - test/nanoFramework.Tool.Tests -> tools/nano/nanoFramework.Tool.Tests ProjectReferences + nanoFramework.Tool.slnx (kept at repo root) repointed. src/ now holds only nanoFramework.NET.Sdk + nanoFramework.Tools.BuildTasks. PackageId/ToolCommandName stay nanoFramework.Tool / nano. Builds, tests (8), and `dotnet pack` (nano tool) verified.
Parallel to tools/nano (the umbrella): each capability gets a named group under tools/. The migrate projects keep their NanoMigrate.* names; only the grouping directory changes. Updated nanoFramework.Tool.slnx paths and the umbrella's ProjectReference. Both solutions build; 108 NanoMigrate + 8 umbrella tests pass; nano tool packs.
Add tools/nano/README.md (the dotnet nano umbrella CLI) and tools/migrate/README.md (the NanoMigrate converter: migrate/clean/rollback/clone/fleet), generated from the plan and verified against each tool's --help. Link both from the repo README. Expand the migration skill with the full command surface (clean, rollback, verify, report, clone/fleet), a commands-at-a-glance table, the rollback-journal/clean workflow, and a --help discovery tip pointing at the READMEs as the source of truth.
The dotnet nano umbrella CLI and the NanoMigrate project migrator, with the companion SDK-migration skill, now live in the nanoframework/nf-tools tools repository. This repository keeps only the MSBuild project SDK. - Remove tools/nano, tools/migrate, the migration skill, and the combined nanoFramework.Tool.slnx. - Point the README at the tools' new home.
(cherry picked from commit 23ed052)
(cherry picked from commit a545128)
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Fail the build with NFSDK001 when the resolved NanoFrameworkMDPVersion is below 4.0.0. The 3.x MetadataProcessor emits the v1 (NFMRK1) assembly format, which cannot represent generics, so a v1 pin would silently produce assemblies that do not load on v2 firmware. The check runs before the MetadataProcessor targets and is skipped when the processor is disabled or its task path is supplied out of band.
Add the NanoDeployableProject capability solely for OutputType=Exe. Library projects are no longer flagged for deploy; their generated .pe is carried in the application deploy bundle. This removes the redundant per-library deploy transactions that raced the debugger during F5 and surfaced as "There were deployment errors".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/nanoFramework.NET.Sdk/Sdk/nanoFramework.Mdp.targets`:
- Around line 36-46: The PackageReference for
nanoFramework.Tools.MetadataProcessor.MsBuildTask contains an explicit Version
attribute that causes the NuGet NU1008 error when projects enable Central
Package Management. Remove the Version="$(NanoFrameworkMDPVersion)" attribute
from the PackageReference element and instead manage the version through the
consuming project's Directory.Packages.props file by adding a PackageVersion
item for nanoFramework.Tools.MetadataProcessor.MsBuildTask, or add documentation
stating that projects using this SDK cannot enable
ManagePackageVersionsCentrally.
In `@src/nanoFramework.NET.Sdk/Sdk/Rules/NanoDebugger.xaml`:
- Around line 17-21: Remove the entire ValueEditor element that contains the
regsvr32.exe configuration from the NanoDebugger.xaml file. The ValueEditor with
EditorType="DefaultFindFullPathPropertyEditor" and the regsvr32.exe metadata
entry should be deleted as it is not relevant to nanoFramework device debugging
and is a leftover artifact from a generic debugger template.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b4bfdf5f-5b85-4ea0-942b-fd27c40dfe8d
📒 Files selected for processing (10)
README.mdsrc/nanoFramework.NET.Sdk/Sdk/Rules/NanoDebugger.xamlsrc/nanoFramework.NET.Sdk/Sdk/Sdk.propssrc/nanoFramework.NET.Sdk/Sdk/Sdk.targetssrc/nanoFramework.NET.Sdk/Sdk/nanoFramework.Capabilities.targetssrc/nanoFramework.NET.Sdk/Sdk/nanoFramework.Mdp.targetssrc/nanoFramework.NET.Sdk/Sdk/nanoFramework.Tfm.propssrc/nanoFramework.NET.Sdk/nanoFramework.NET.Sdk.csprojsrc/nanoFramework.Tools.BuildTasks/nanoFramework.Tools.BuildTasks.csprojtest/SmokeTest/NuGet.Config
nanoFramework.Mdp.targets had become an umbrella for the whole nanoFramework build pipeline. Move the non-MetadataProcessor concerns into their own modules: nanoFramework.Resources.targets (GenerateNanoResourceTask + NanoResourceGenerator, the .resx -> .nanoresources PrepareResources step) and nanoFramework.Output.targets (GenerateBinaryOutputTask, NanoGenerateBinaryOutput, the artifact copy targets, and the clean targets). Mdp.targets keeps the MetadataProcessor proper plus the build-phase orchestration and the shared $(_NfSdkTasksDir) / $(NanoFramework_*) properties, so it is imported first. Pure relocation: every target, task, and property is preserved (verified). Sdk.targets imports the new modules and the package packs them.
josesimoes
changed the title
- If devs are messing with the SDK files they better know what they are doing.
josesimoes
reviewed
Jun 24, 2026
josesimoes
reviewed
Jun 24, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/nanoFramework.NET.Sdk/Sdk/Sdk.targets (1)
71-76: 🗄️ Data Integrity & Integration | 🟡 MinorAdd
NuGetTargetMonikerto the targets-phase TFM override block for consistency.The props-phase assignment in
nanoFramework.Tfm.propssetsNuGetTargetMonikertogether with the other TFM identity properties. However, the targets-phase override block (lines 71–76) re-assertsTargetFrameworkIdentifier,TargetFrameworkVersion,TargetFrameworkMoniker, andTargetFrameworkMonikerDisplayName, but omitsNuGetTargetMoniker.Since Microsoft.NET.Sdk's
Microsoft.PackageDependencyResolution.targetscomputesNuGetTargetMonikerbased onTargetFrameworkMonikerduring the targets phase, the absence of an override could leaveNuGetTargetMonikerset to the SDK-inferred value rather than the corrected.NETnanoFramework,Version=$(TargetFrameworkVersion), affecting NuGet restore and pack operations. Either addNuGetTargetMonikerto the override block or document why it is safe to omit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nanoFramework.NET.Sdk/Sdk/Sdk.targets` around lines 71 - 76, In the PropertyGroup block that redefines the target framework properties (TargetFrameworkIdentifier, TargetFrameworkVersion, TargetFrameworkMoniker, and TargetFrameworkMonikerDisplayName), add a new property for NuGetTargetMoniker with the value .NETnanoFramework,Version=$(TargetFrameworkVersion) to match the pattern used in the props-phase assignment and ensure consistency across NuGet restore and pack operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/nanoFramework.NET.Sdk/Sdk/nanoFramework.Output.targets`:
- Around line 45-47: In the Copy task element for CopyToOutDir, remove the
$(ProjectDir) prefix from the path construction in both the Condition attribute
and the SourceFiles attribute. Replace
$(ProjectDir)$(IntermediateOutputPath)$(TargetFileName) with just
$(IntermediateOutputPath)$(TargetFileName) in both locations, since
IntermediateOutputPath can be an absolute path and concatenating ProjectDir to
it creates an invalid path. This pattern should match the correct implementation
used elsewhere in the file.
In `@src/nanoFramework.NET.Sdk/Sdk/nanoFramework.Resources.targets`:
- Line 39: In the OutputResources attribute on line 39, remove the $(ProjectDir)
prefix from the path string. The OutputResources path should use only
$(IntermediateOutputPath)%(ManifestResourceName).nanoresources without the
preceding $(ProjectDir) component, matching the pattern used in the StateFile
attribute on line 38. This prevents path resolution issues when
IntermediateOutputPath is configured as an absolute path.
---
Outside diff comments:
In `@src/nanoFramework.NET.Sdk/Sdk/Sdk.targets`:
- Around line 71-76: In the PropertyGroup block that redefines the target
framework properties (TargetFrameworkIdentifier, TargetFrameworkVersion,
TargetFrameworkMoniker, and TargetFrameworkMonikerDisplayName), add a new
property for NuGetTargetMoniker with the value
.NETnanoFramework,Version=$(TargetFrameworkVersion) to match the pattern used in
the props-phase assignment and ensure consistency across NuGet restore and pack
operations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7d959a41-bb71-4eb0-959a-820673a03804
📒 Files selected for processing (9)
src/nanoFramework.NET.Sdk/Sdk/Rules/NanoDebugger.xamlsrc/nanoFramework.NET.Sdk/Sdk/Sdk.propssrc/nanoFramework.NET.Sdk/Sdk/Sdk.targetssrc/nanoFramework.NET.Sdk/Sdk/nanoFramework.Capabilities.targetssrc/nanoFramework.NET.Sdk/Sdk/nanoFramework.Mdp.targetssrc/nanoFramework.NET.Sdk/Sdk/nanoFramework.Output.targetssrc/nanoFramework.NET.Sdk/Sdk/nanoFramework.Resources.targetssrc/nanoFramework.NET.Sdk/Sdk/nanoFramework.Tfm.propssrc/nanoFramework.NET.Sdk/nanoFramework.NET.Sdk.csproj
Member
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Member
|
Pipeline failing on missing source files for companion DLLs. Expected as this is a work in progress. Merging anyway. |
josesimoes
approved these changes
Jun 24, 2026
josesimoes
left a comment
josesimoes
left a comment
Member
There was a problem hiding this comment.
Nice improvements.
I've added several other changes on top of your work. Added those to the PR description.
This was referenced Jun 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
DebugType=fullbefore theMicrosoft.NET.Sdkimport, so Visual Studio binds source breakpoints instead of stopping only at the method entry.LaunchProfilesproject capability so the C# project system's launcher does not own F5.Sdk/Rules/NanoDebugger.xamldebugger rule and register it throughPropertyPageSchemaso the debugger property page is available.Sdk.targetsinto a dedicatednanoFramework.Mdp.targets, and move the target framework identity and compiler defaults intonanoFramework.Tfm.props, leavingSdk.targetsas a thin orchestrator.4.0.0-preview.94(which emits theNFMRK2PE format) and setting the MDP task target framework tonet8.0for the Core MSBuild runtime.Motivation and Context
How Has This Been Tested?
test/SmokeTestunder Visual Studio MSBuild: the output PE magic isNFMRK2and the Debug.pdbmagic isMicrosoft C/C++ MSF(a Windows/full PDB, not a portableBSJBPDB).Types of changes
Checklist: