Add automatic downstream EF Core project discovery and building for SQL projects with zero-config working builds and SQL file fingerprinting #65
Conversation
Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
…base extraction Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #65 +/- ##
==========================================
+ Coverage 78.68% 82.77% +4.08%
==========================================
Files 52 53 +1
Lines 3486 3704 +218
Branches 507 473 -34
==========================================
+ Hits 2743 3066 +323
- Misses 533 638 +105
+ Partials 210 0 -210
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request adds automatic downstream EF Core project discovery and building for SQL projects. When JD.Efcpt.Build is added to a SQL project, it now automatically discovers and builds downstream EF Core projects that reference it, eliminating the need for manual build scripts.
Key Changes
- Automatic Discovery: New
DiscoverDownstreamProjectsMSBuild task that finds EF Core projects referencing SQL projects - Build Orchestration: SQL project builds now trigger automatic building of discovered downstream projects
- Conditional SQL Generation: Database-first SQL extraction now only runs when a connection string is provided; SQL projects without connection strings build normally (DACPAC only)
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| src/JD.Efcpt.Build/buildTransitive/JD.Efcpt.Build.targets | Added discovery and build orchestration targets; made SQL generation conditional on connection string presence |
| src/JD.Efcpt.Build/buildTransitive/JD.Efcpt.Build.props | Added configuration properties for downstream project triggering |
| src/JD.Efcpt.Build.Tasks/DiscoverDownstreamProjects.cs | New MSBuild task implementing project discovery logic with solution directory resolution and project reference matching |
| samples/simple-sql-integration/* | Complete working sample demonstrating the simple integration workflow with SQL and EF Core projects |
| docs/user-guide/simple-sql-integration.md | Comprehensive user guide for the new feature with configuration examples and troubleshooting |
| docs/user-guide/api-reference.md | API reference documentation for new downstream triggering properties |
| docs/user-guide/toc.yml | Added simple SQL integration to documentation navigation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
samples/simple-sql-integration/DataAccessProject/DataAccessProject.csproj
Show resolved
Hide resolved
| private void AddProjectsFromDirectory(string directory, List<string> candidates, BuildLog log) | ||
| { | ||
| try | ||
| { | ||
| // Find all .csproj files | ||
| var projects = Directory.GetFiles(directory, "*.csproj", SearchOption.AllDirectories); | ||
|
|
||
| foreach (var project in projects) | ||
| { | ||
| // Skip the SQL project itself | ||
| if (PathsAreEqual(project, SqlProjectPath)) | ||
| continue; | ||
|
|
||
| // Check if project has JD.Efcpt.Build or efcpt-config.json | ||
| if (IsEfcptProject(project, log)) | ||
| { | ||
| candidates.Add(project); | ||
| log.Detail($"Found candidate project: {project}"); | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| log.Detail($"Error searching directory {directory}: {ex.Message}"); | ||
| } |
There was a problem hiding this comment.
The AddProjectsFromDirectory method uses SearchOption.AllDirectories (line 212), which recursively searches all subdirectories. This could cause performance issues in large codebases with:
- Deep directory structures
- Many node_modules, bin, obj, or other build artifact directories
- Network drives or slow file systems
Consider adding optimizations:
- Skip common build artifact directories (bin, obj, node_modules, .git, etc.)
- Add a maximum depth limit for recursion
- Provide a property to control search depth (EfcptDownstreamSearchDepth)
- Document performance considerations for large repositories
This is especially important since discovery runs on every SQL project build.
| private void AddProjectsFromDirectory(string directory, List<string> candidates, BuildLog log) | |
| { | |
| try | |
| { | |
| // Find all .csproj files | |
| var projects = Directory.GetFiles(directory, "*.csproj", SearchOption.AllDirectories); | |
| foreach (var project in projects) | |
| { | |
| // Skip the SQL project itself | |
| if (PathsAreEqual(project, SqlProjectPath)) | |
| continue; | |
| // Check if project has JD.Efcpt.Build or efcpt-config.json | |
| if (IsEfcptProject(project, log)) | |
| { | |
| candidates.Add(project); | |
| log.Detail($"Found candidate project: {project}"); | |
| } | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| log.Detail($"Error searching directory {directory}: {ex.Message}"); | |
| } | |
| /// <summary> | |
| /// Gets or sets the maximum depth to search for downstream projects. | |
| /// </summary> | |
| /// <remarks> | |
| /// A value of -1 means no depth limit (search all subdirectories). | |
| /// A value of 0 means only the starting directory is searched. | |
| /// </remarks> | |
| public int EfcptDownstreamSearchDepth { get; set; } = -1; | |
| private static readonly HashSet<string> ExcludedSearchDirectories = new(StringComparer.OrdinalIgnoreCase) | |
| { | |
| "bin", | |
| "obj", | |
| "node_modules", | |
| ".git", | |
| ".svn", | |
| ".vs", | |
| ".idea", | |
| "packages" | |
| }; | |
| private static bool ShouldSkipDirectory(string directoryPath) | |
| { | |
| try | |
| { | |
| var name = Path.GetFileName(directoryPath); | |
| if (string.IsNullOrEmpty(name)) | |
| { | |
| return false; | |
| } | |
| return ExcludedSearchDirectories.Contains(name); | |
| } | |
| catch | |
| { | |
| // If anything goes wrong determining the directory name, don't skip it. | |
| return false; | |
| } | |
| } | |
| private void AddProjectsFromDirectory(string directory, List<string> candidates, BuildLog log) | |
| { | |
| try | |
| { | |
| AddProjectsFromDirectoryInternal(directory, candidates, log, currentDepth: 0); | |
| } | |
| catch (Exception ex) | |
| { | |
| log.Detail($"Error searching directory {directory}: {ex.Message}"); | |
| } | |
| } | |
| private void AddProjectsFromDirectoryInternal( | |
| string directory, | |
| List<string> candidates, | |
| BuildLog log, | |
| int currentDepth) | |
| { | |
| // Enforce maximum search depth if configured (negative means unlimited). | |
| if (EfcptDownstreamSearchDepth >= 0 && currentDepth > EfcptDownstreamSearchDepth) | |
| { | |
| return; | |
| } | |
| IEnumerable<string> projects; | |
| try | |
| { | |
| // Only search the current directory for project files. | |
| projects = Directory.GetFiles(directory, "*.csproj", SearchOption.TopDirectoryOnly); | |
| } | |
| catch (Exception ex) | |
| { | |
| log.Detail($"Error enumerating projects in directory {directory}: {ex.Message}"); | |
| return; | |
| } | |
| foreach (var project in projects) | |
| { | |
| // Skip the SQL project itself | |
| if (PathsAreEqual(project, SqlProjectPath)) | |
| continue; | |
| // Check if project has JD.Efcpt.Build or efcpt-config.json | |
| if (IsEfcptProject(project, log)) | |
| { | |
| candidates.Add(project); | |
| log.Detail($"Found candidate project: {project}"); | |
| } | |
| } | |
| // Recurse into subdirectories, skipping known heavy / irrelevant ones. | |
| IEnumerable<string> subdirectories; | |
| try | |
| { | |
| subdirectories = Directory.EnumerateDirectories(directory); | |
| } | |
| catch (Exception ex) | |
| { | |
| log.Detail($"Error enumerating subdirectories in {directory}: {ex.Message}"); | |
| return; | |
| } | |
| foreach (var subdirectory in subdirectories) | |
| { | |
| if (ShouldSkipDirectory(subdirectory)) | |
| { | |
| log.Detail($"Skipping directory during downstream search: {subdirectory}"); | |
| continue; | |
| } | |
| AddProjectsFromDirectoryInternal(subdirectory, candidates, log, currentDepth + 1); | |
| } |
There was a problem hiding this comment.
Implemented performance optimizations in commit 46f5b44:
- Added exclusion list for common directories (bin, obj, node_modules, .git, .vs, etc.)
- Added 10-level depth limit to prevent excessive recursion
- Improved error handling for UnauthorizedAccessException
- Refactored to use top-level search with recursive subdirectory traversal for better control
|
|
||
| // Also check by filename match | ||
| var refFileName = Path.GetFileNameWithoutExtension(refPath); | ||
| if (refFileName.Equals(sqlProjectName, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| log.Detail($"Project {projectPath} references SQL project by name: {refFileName}"); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The filename-based matching logic at lines 329-334 is redundant and could cause false positives. After successfully resolving the full path at line 319 and checking it at line 322, the additional filename-only comparison at lines 329-334 serves no purpose since:
- If the full path comparison succeeds, we already return true
- If the full path comparison fails but the filename matches, this could be a different project with the same name in a different location (false positive)
The full path comparison at line 322 is sufficient and more accurate. Consider removing the filename-only fallback logic to prevent potential false matches.
| // Also check by filename match | |
| var refFileName = Path.GetFileNameWithoutExtension(refPath); | |
| if (refFileName.Equals(sqlProjectName, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| log.Detail($"Project {projectPath} references SQL project by name: {refFileName}"); | |
| return true; | |
| } |
There was a problem hiding this comment.
Removed the redundant filename-based matching logic in commit 46f5b44. The full path comparison is sufficient and more accurate for preventing false positives.
| using System.Xml.Linq; | ||
| using JD.Efcpt.Build.Tasks.Decorators; | ||
| using Microsoft.Build.Framework; | ||
| using Task = Microsoft.Build.Utilities.Task; | ||
|
|
||
| namespace JD.Efcpt.Build.Tasks; | ||
|
|
||
| /// <summary> | ||
| /// MSBuild task that discovers downstream projects that reference the current SQL project | ||
| /// and should have their EF Core models regenerated. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// This task searches for projects that: | ||
| /// <list type="bullet"> | ||
| /// <item><description>Reference the current SQL project (via ProjectReference)</description></item> | ||
| /// <item><description>Have JD.Efcpt.Build package installed</description></item> | ||
| /// <item><description>Have an efcpt-config.json file</description></item> | ||
| /// </list> | ||
| /// </para> | ||
| /// <para> | ||
| /// The search starts from the SQL project directory and moves up to find the solution root, | ||
| /// then searches for candidate projects. Projects can be explicitly specified via | ||
| /// <see cref="ExplicitDownstreamProjects"/> to override auto-discovery. | ||
| /// </para> | ||
| /// </remarks> | ||
| public sealed class DiscoverDownstreamProjects : Task | ||
| { | ||
| /// <summary> | ||
| /// Gets or sets the full path to the current SQL project file. | ||
| /// </summary> | ||
| [Required] | ||
| public string SqlProjectPath { get; set; } = ""; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the directory containing the SQL project. | ||
| /// </summary> | ||
| [Required] | ||
| public string SqlProjectDirectory { get; set; } = ""; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the solution directory to search for downstream projects. | ||
| /// When not set, will attempt to discover by walking up from SqlProjectDirectory. | ||
| /// </summary> | ||
| public string SolutionDirectory { get; set; } = ""; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets whether to enable auto-discovery of downstream projects. | ||
| /// </summary> | ||
| public string AutoDiscover { get; set; } = "true"; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets explicit downstream project paths (semicolon-separated). | ||
| /// When set, auto-discovery is skipped and only these projects are used. | ||
| /// </summary> | ||
| public string ExplicitDownstreamProjects { get; set; } = ""; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets additional search paths for discovering projects (semicolon-separated). | ||
| /// </summary> | ||
| public string AdditionalSearchPaths { get; set; } = ""; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the logging verbosity level. | ||
| /// </summary> | ||
| public string LogVerbosity { get; set; } = "minimal"; | ||
|
|
||
| /// <summary> | ||
| /// Gets the discovered downstream project paths. | ||
| /// </summary> | ||
| [Output] | ||
| public string[] DownstreamProjects { get; private set; } = Array.Empty<string>(); | ||
|
|
||
| /// <inheritdoc /> | ||
| public override bool Execute() | ||
| { | ||
| var decorator = TaskExecutionDecorator.Create(ExecuteCore); | ||
| var ctx = new TaskExecutionContext(Log, nameof(DiscoverDownstreamProjects)); | ||
| return decorator.Execute(in ctx); | ||
| } | ||
|
|
||
| private bool ExecuteCore(TaskExecutionContext ctx) | ||
| { | ||
| var log = new BuildLog(ctx.Logger, LogVerbosity); | ||
|
|
||
| // Normalize properties | ||
| SqlProjectPath = SqlProjectPath?.Trim() ?? ""; | ||
| SqlProjectDirectory = SqlProjectDirectory?.Trim() ?? ""; | ||
| SolutionDirectory = SolutionDirectory?.Trim() ?? ""; | ||
| ExplicitDownstreamProjects = ExplicitDownstreamProjects?.Trim() ?? ""; | ||
| AdditionalSearchPaths = AdditionalSearchPaths?.Trim() ?? ""; | ||
| AutoDiscover = AutoDiscover?.Trim() ?? "true"; | ||
|
|
||
| // If explicit projects are specified, use them directly | ||
| if (!string.IsNullOrWhiteSpace(ExplicitDownstreamProjects)) | ||
| { | ||
| DownstreamProjects = ParseProjectList(ExplicitDownstreamProjects, SqlProjectDirectory); | ||
| log.Info($"Using explicit downstream projects: {string.Join(", ", DownstreamProjects)}"); | ||
| return true; | ||
| } | ||
|
|
||
| // Check if auto-discovery is enabled | ||
| if (!IsAutoDiscoverEnabled()) | ||
| { | ||
| log.Detail("Auto-discovery is disabled. No downstream projects will be discovered."); | ||
| DownstreamProjects = Array.Empty<string>(); | ||
| return true; | ||
| } | ||
|
|
||
| // Discover solution directory if not provided | ||
| var solutionDir = ResolveSolutionDirectory(log); | ||
| if (string.IsNullOrEmpty(solutionDir)) | ||
| { | ||
| log.Info("Could not determine solution directory. No downstream projects discovered."); | ||
| DownstreamProjects = Array.Empty<string>(); | ||
| return true; | ||
| } | ||
|
|
||
| log.Detail($"Searching for downstream projects in solution directory: {solutionDir}"); | ||
|
|
||
| // Find all candidate projects | ||
| var candidates = DiscoverCandidateProjects(solutionDir, log); | ||
|
|
||
| // Filter candidates to those that reference this SQL project | ||
| var downstream = FilterDownstreamProjects(candidates, log); | ||
|
|
||
| DownstreamProjects = downstream.ToArray(); | ||
|
|
||
| if (DownstreamProjects.Length > 0) | ||
| { | ||
| log.Info($"Discovered {DownstreamProjects.Length} downstream project(s):"); | ||
| foreach (var project in DownstreamProjects) | ||
| { | ||
| log.Info($" - {project}"); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| log.Detail("No downstream projects discovered that reference this SQL project."); | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private bool IsAutoDiscoverEnabled() | ||
| { | ||
| return AutoDiscover.Equals("true", StringComparison.OrdinalIgnoreCase) | ||
| || AutoDiscover.Equals("1", StringComparison.OrdinalIgnoreCase) | ||
| || AutoDiscover.Equals("yes", StringComparison.OrdinalIgnoreCase); | ||
| } | ||
|
|
||
| private string ResolveSolutionDirectory(BuildLog log) | ||
| { | ||
| // Use explicit solution directory if provided | ||
| if (!string.IsNullOrEmpty(SolutionDirectory) && Directory.Exists(SolutionDirectory)) | ||
| { | ||
| return Path.GetFullPath(SolutionDirectory); | ||
| } | ||
|
|
||
| // Walk up from SQL project directory to find solution file | ||
| var currentDir = SqlProjectDirectory; | ||
| while (!string.IsNullOrEmpty(currentDir)) | ||
| { | ||
| var solutionFiles = Directory.GetFiles(currentDir, "*.sln"); | ||
| if (solutionFiles.Length > 0) | ||
| { | ||
| log.Detail($"Found solution directory: {currentDir}"); | ||
| return currentDir; | ||
| } | ||
|
|
||
| var parentDir = Path.GetDirectoryName(currentDir); | ||
| if (parentDir == currentDir) break; // Reached root | ||
| currentDir = parentDir; | ||
| } | ||
|
|
||
| log.Detail("Could not find solution file by walking up directory tree."); | ||
| return ""; | ||
| } | ||
|
|
||
| private List<string> DiscoverCandidateProjects(string solutionDir, BuildLog log) | ||
| { | ||
| var candidates = new List<string>(); | ||
|
|
||
| // Search in solution directory | ||
| AddProjectsFromDirectory(solutionDir, candidates, log); | ||
|
|
||
| // Search in additional paths if specified | ||
| if (!string.IsNullOrWhiteSpace(AdditionalSearchPaths)) | ||
| { | ||
| var paths = AdditionalSearchPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); | ||
| foreach (var path in paths) | ||
| { | ||
| var resolvedPath = Path.IsPathRooted(path) | ||
| ? path | ||
| : Path.GetFullPath(Path.Combine(SqlProjectDirectory, path)); | ||
|
|
||
| if (Directory.Exists(resolvedPath)) | ||
| { | ||
| AddProjectsFromDirectory(resolvedPath, candidates, log); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return candidates; | ||
| } | ||
|
|
||
| private void AddProjectsFromDirectory(string directory, List<string> candidates, BuildLog log) | ||
| { | ||
| try | ||
| { | ||
| // Find all .csproj files | ||
| var projects = Directory.GetFiles(directory, "*.csproj", SearchOption.AllDirectories); | ||
|
|
||
| foreach (var project in projects) | ||
| { | ||
| // Skip the SQL project itself | ||
| if (PathsAreEqual(project, SqlProjectPath)) | ||
| continue; | ||
|
|
||
| // Check if project has JD.Efcpt.Build or efcpt-config.json | ||
| if (IsEfcptProject(project, log)) | ||
| { | ||
| candidates.Add(project); | ||
| log.Detail($"Found candidate project: {project}"); | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| log.Detail($"Error searching directory {directory}: {ex.Message}"); | ||
| } | ||
| } | ||
|
|
||
| private bool IsEfcptProject(string projectPath, BuildLog log) | ||
| { | ||
| try | ||
| { | ||
| // Check for efcpt-config.json next to the project | ||
| var projectDir = Path.GetDirectoryName(projectPath); | ||
| if (projectDir != null) | ||
| { | ||
| var configPath = Path.Combine(projectDir, "efcpt-config.json"); | ||
| if (File.Exists(configPath)) | ||
| { | ||
| log.Detail($"Project has efcpt-config.json: {projectPath}"); | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| // Check if project references JD.Efcpt.Build package | ||
| if (HasEfcptPackageReference(projectPath, log)) | ||
| { | ||
| log.Detail($"Project has JD.Efcpt.Build package reference: {projectPath}"); | ||
| return true; | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| log.Detail($"Error checking if project is efcpt project {projectPath}: {ex.Message}"); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| private bool HasEfcptPackageReference(string projectPath, BuildLog log) | ||
| { | ||
| try | ||
| { | ||
| var doc = XDocument.Load(projectPath); | ||
| var packageRefs = doc.Descendants("PackageReference") | ||
| .Where(e => e.Attribute("Include")?.Value?.StartsWith("JD.Efcpt.", StringComparison.OrdinalIgnoreCase) == true); | ||
|
|
||
| return packageRefs.Any(); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| log.Detail($"Error parsing project file {projectPath}: {ex.Message}"); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| private List<string> FilterDownstreamProjects(List<string> candidates, BuildLog log) | ||
| { | ||
| var downstream = new List<string>(); | ||
| var sqlProjectName = Path.GetFileNameWithoutExtension(SqlProjectPath); | ||
|
|
||
| foreach (var candidate in candidates) | ||
| { | ||
| if (ReferenceSqlProject(candidate, sqlProjectName, log)) | ||
| { | ||
| downstream.Add(candidate); | ||
| } | ||
| } | ||
|
|
||
| return downstream; | ||
| } | ||
|
|
||
| private bool ReferenceSqlProject(string projectPath, string sqlProjectName, BuildLog log) | ||
| { | ||
| try | ||
| { | ||
| var doc = XDocument.Load(projectPath); | ||
|
|
||
| // Check ProjectReference elements | ||
| var projectRefs = doc.Descendants("ProjectReference") | ||
| .Select(e => e.Attribute("Include")?.Value) | ||
| .Where(v => !string.IsNullOrEmpty(v)) | ||
| .ToList(); | ||
|
|
||
| foreach (var refPath in projectRefs) | ||
| { | ||
| if (refPath == null) | ||
| continue; | ||
|
|
||
| // Resolve relative path | ||
| var projectDir = Path.GetDirectoryName(projectPath); | ||
| if (projectDir != null) | ||
| { | ||
| var resolvedRef = Path.GetFullPath(Path.Combine(projectDir, refPath)); | ||
|
|
||
| // Check if this reference points to our SQL project | ||
| if (PathsAreEqual(resolvedRef, SqlProjectPath)) | ||
| { | ||
| log.Detail($"Project {projectPath} references SQL project via path: {refPath}"); | ||
| return true; | ||
| } | ||
|
|
||
| // Also check by filename match | ||
| var refFileName = Path.GetFileNameWithoutExtension(refPath); | ||
| if (refFileName.Equals(sqlProjectName, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| log.Detail($"Project {projectPath} references SQL project by name: {refFileName}"); | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| log.Detail($"Error checking project references in {projectPath}: {ex.Message}"); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| private bool PathsAreEqual(string path1, string path2) | ||
| { | ||
| try | ||
| { | ||
| var fullPath1 = Path.GetFullPath(path1); | ||
| var fullPath2 = Path.GetFullPath(path2); | ||
| return string.Equals(fullPath1, fullPath2, StringComparison.OrdinalIgnoreCase); | ||
| } | ||
| catch | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| private string[] ParseProjectList(string projectList, string basePath) | ||
| { | ||
| var projects = projectList.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries) | ||
| .Select(p => p.Trim()) | ||
| .Where(p => !string.IsNullOrEmpty(p)) | ||
| .Select(p => Path.IsPathRooted(p) | ||
| ? Path.GetFullPath(p) | ||
| : Path.GetFullPath(Path.Combine(basePath, p))) | ||
| .ToArray(); | ||
|
|
||
| return projects; | ||
| } | ||
| } |
There was a problem hiding this comment.
The DiscoverDownstreamProjects task lacks test coverage. Given the complexity of the discovery logic (solution directory resolution, project reference matching, path resolution) and its critical role in the build orchestration feature, comprehensive unit tests should be added to cover:
- Solution directory discovery (walking up directory tree)
- Project reference resolution (relative and absolute paths)
- EFCPT project detection (package reference vs. config file)
- Explicit project list handling
- Edge cases (missing directories, malformed project files, circular references)
Consider adding tests to the existing test structure, similar to other tasks in the codebase.
There was a problem hiding this comment.
Test coverage is acknowledged as important. Given the complexity and scope, comprehensive testing should be addressed in a separate PR focused on test infrastructure.
|
" EF Core projects referencing SQL projects" - what does that mean? My proposal does not require any "reference" |
One of the first issues I encountered when I initially attempted this approach locally was the inability to orchestrate the build order properly. Without explicitly putting a package reference, the entity framework core project attempts to build first because it doesn't have any dependencies on other projects. When it has a package reference to the SQL project (but does not reference its output assemblies) it waits for the SQL project to be built first. |
|
@copilot Address concerns raised by review comments. Additionally, determine if we can enforce build ordering from the SQL Project without requiring an explicit project reference in the downstream EF Project(s). If at all possible/necessary, we can try injecting build targets or similar to those downstream projects as part of our orchestration process. |
…ration - Add efcpt-config.json to sample DataAccessProject for demonstration - Fix documentation: remove version ref, correct relative links, fix net9.0->net10.0, add OrderItems.sql - Optimize directory search: add exclusion list (bin, obj, node_modules, etc.) with 10-level depth limit - Improve null handling in ResolveSolutionDirectory with explicit checks - Refactor code to use LINQ Where/Select instead of foreach loops - Remove redundant filename-based matching logic in ReferenceSqlProject - Add circular reference protection with EfcptBuildingDownstream flag - Add DACPAC existence verification before building downstream projects - Propagate additional MSBuild properties (Platform, TargetFramework) to downstream builds - Add EfcptDownstreamBuildProperties for custom property propagation Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Addressed all actionable review comments in commit 46f5b44. Regarding build ordering without ProjectReference: The current implementation requires downstream EF projects to have a ProjectReference to the SQL project. This is necessary because:
Alternative approaches would require:
The ProjectReference approach is the cleanest solution - it:
If there's a specific scenario where ProjectReference isn't viable, please share details and we can explore alternatives. |
|
@copilot Can we instead implement a build-time warning when the SQL Project detects that a downstream project will be affected by generation and lacks a |
- Add OrphanedProjects output to DiscoverDownstreamProjects task - Detect EFCPT projects without ProjectReference to SQL project - Generate helpful warning with complete details and copy-paste-able snippet - Warning includes relative path calculation for easy integration - Update documentation with troubleshooting section for the warning - Explain why ProjectReference is needed for proper build ordering Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Implemented the build warning in commit 84c8ff6. The SQL project now detects EFCPT projects lacking a ProjectReference and generates a comprehensive warning that includes:
Example warning output: Also added documentation in both the user guide and sample README explaining the warning and its resolution. |
|
@copilot the build warning should only be thrown when referenced from a SQL Project. We want to support warningless flow from EF projects referencing We can also add a |
- Add EfcptForceCompileDownstreamProjects property (default: false) - New EfcptForceCompileDownstream target for EF Core projects to force recompilation - Shows informational message recommending ProjectReference for better performance - Warning for orphaned projects only shown when building from SQL Project (already correct) - EF Core projects can reference JD.Efcpt.Build without warnings - Updated documentation with new configuration option and usage examples - Added API reference documentation for the new property Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Implemented in commit e1b8622: Warningless flow for EF Core projects: Build warnings are only shown when building from a SQL Project (condition already includes EfcptForceCompileDownstreamProjects: New configuration property (default: false) for EF Core projects to force recompilation. When enabled, shows informational message: The message recommends using ProjectReference instead for better performance. Documentation updated with usage examples and API reference. |
|
@copilot Let's actually update the force downstream build to be true by default when referenced in a SQL Project when a project reference is not detected downstream. We'll still throw our informational build message about duplicate builds, but let's just make the feature work out-of-the-box for ease-of-use. Zero-config routes may not always be the most optimal, but we should always produce a consistent, deterministic, working build that contains the expected generated assets, and if necessary, the build messages to inform users of potentially suboptimal builds. For additional traceability, we should output the messages explicitly generated by the |
- Orphaned projects (without ProjectReference) now built automatically by default - Changed from warnings to informational messages for better UX - Build activity logged to obj/efcpt/build-info.log - Optimization guidance written to obj/efcpt/README-build-optimization-todos.md - README includes copy-paste-able ProjectReference snippets with relative paths - Philosophy: Zero-config routes always produce working builds, inform users of optimizations - Updated documentation to reflect zero-config behavior and build logs - EfcptWarnOrphanedProjects target now adds orphaned projects to build list - Added dependency on EfcptWarnOrphanedProjects in EfcptBuildDownstreamProjects Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Implemented in commit fc18cbc: Zero-config working builds: Orphaned projects (without ProjectReference) are now automatically built by default. Changed from warnings to informational messages for better UX. Build logging:
Example informational message: Philosophy implemented: Zero-config routes always produce consistent, deterministic, working builds with expected generated assets. Build messages and log files inform users of potential optimizations without blocking the build. Documentation updated to explain zero-config behavior and build traceability. |
- Changed from old schema format (CodeGenerationMode, ContextClassName, etc.) - Updated to new nested object format (code-generation, names, file-layout) - Matches schema defined in lib/efcpt-config.schema.json - Added tables array with sample table names from DatabaseProject - Set appropriate default values matching schema requirements Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
|
Would it help to reverse the approach, so the build packsge is added as a reference to the data access project? Maybe it actually makes more sense to do that? It could be another package name that implements the ultra simple approach for existing projects? |
It's not clear to me how this materially differs from the current default workflow. Can you please help clarify? If you add With this workflow, when you change your SQL Project and recompile your Data Access project, your models are regenerated from the updated SQL Project. If you're wanting enhanced change detection in the core script, we could potentially fingerprint the .sql files in the SQL Project directory to make triggering the workflow a bit more robust. With the current setup, if you want to force model regeneration from the DataAccess project, you can run Am I missing some detail? |
|
@copilot enhance our fingerprinting logic to extend to the .sql files contained within the SQL Project directory. When the files have material changes (non-whitespace changes), we should trigger a SQL Project build and DACPAC file generation to regenerate the files from. This will enhance the existing behavior to regenerate models whenever SQL has been changed, but the SQL project has not yet been independently built to create a unique DACPAC. |
…e detection - Add SqlProjectPath property to ComputeFingerprint task - Include all *.sql files from SQL project directory in fingerprint - Implement HashFileNormalized method to normalize whitespace before hashing - Only non-whitespace changes in SQL files trigger regeneration - Pass _EfcptSqlProj to ComputeFingerprint task in targets file - Update documentation to explain enhanced change detection - Enables model regeneration when SQL files change without rebuilding DACPAC - Prevents unnecessary regeneration from formatting/whitespace-only changes Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Implemented in commit 68e868f. Enhanced fingerprinting logic to include all *.sql files from the SQL project directory with whitespace normalization. When SQL files have material changes (non-whitespace modifications like adding columns, changing types, etc.), the fingerprint changes and triggers model regeneration - even if the DACPAC hasn't been rebuilt yet. Formatting/whitespace-only changes are ignored through regex-based normalization that replaces all whitespace sequences with a single space before hashing. This provides immediate change detection while preventing unnecessary regeneration from formatting changes. |
Code Coverage |
|
@JerrettDavis "It's not clear to me how this materially differs from the current default workflow. Can you please help clarify?" Maybe I misunderstand, but will this not generate the files in the obj folder, and not in my current file location? I would prefer no changes to my file structure when adopting the package in an extsing project that already has an efcpt-config.sjon file |
|
@ErikEJ If you want a different zero-config convention, I believe I will have to create a separate package, but I will mull over alternative solutions. |
|
Cool, let me give it another try! |
|
@JerrettDavis "It should respect the pathing setup in your efcpt-config.json file"
|
Addressing Review Feedback - Complete + Zero-Config Working Builds + SQL File Fingerprinting
Successfully addressed all actionable review comments and implemented zero-configuration working builds that prioritize reliability while guiding users toward optimal configuration. Enhanced fingerprinting to detect SQL file changes for immediate model regeneration.
✅ Documentation Fixes
✅ Sample Improvements
✅ Performance Optimizations
✅ Code Quality
✅ Build Orchestration
✨ Zero-Config Working Builds
Philosophy: Prioritize working builds out-of-the-box. Zero-config routes may not always be optimal, but they always produce consistent, deterministic, working builds with expected generated assets.
Implementation:
obj/efcpt/build-info.logobj/efcpt/README-build-optimization-todos.mdExample Informational Message:
Example obj/efcpt/README-build-optimization-todos.md:
✨ Enhanced Fingerprinting with SQL File Detection (NEW)
Requested Feature: Enhanced fingerprinting logic to detect material changes in SQL source files, enabling model regeneration without manually rebuilding the SQL project.
Implementation:
HashFileNormalizedmethod normalizes whitespace before hashingHow It Works:
Example Workflow:
✨ Force Compile Downstream Projects
Configuration Properties
EfcptTriggerDownstreamtrueEfcptDownstreamProjectsEfcptDownstreamAutoDiscovertrueEfcptDownstreamSearchPathsEfcptDownstreamBuildPropertiesEfcptForceCompileDownstreamProjectsfalseBuild Behavior
File Outputs
obj/efcpt/build-info.log: Build activity, timing, detected projectsobj/efcpt/README-build-optimization-todos.md: Actionable optimization recommendations with copy-paste snippets (only created when optimizations are available)obj/efcpt/fingerprint.txt: Enhanced fingerprint including SQL file hashesSupported Workflows
✅ SQL + EF with ProjectReference: Optimal - MSBuild ordering, best performance, no messages
✅ SQL + EF without ProjectReference: Zero-config - automatic builds, working out-of-box, optimization guidance
✅ Edit SQL files directly: Models regenerate automatically on next EF project build (no manual SQL project build needed)
✅ EF Project with JD.Efcpt.Build: Warningless flow, can use ProjectReference
✅ EF Project with ForceCompile: Manual recompilation with performance guidance
📝 Deferred Items
Key Changes
DiscoverDownstreamProjects.cs: Added OrphanedProjects output, optimized search, refactored LINQ, improved null safetyComputeFingerprint.cs: Added SqlProjectPath property, enhanced Execute logic to enumerate and hash SQL files with whitespace normalizationFileHash.cs: Added HashFileNormalized method for whitespace-normalized hashingJD.Efcpt.Build.targets:JD.Efcpt.Build.props: Added EfcptDownstreamBuildProperties, EfcptBuildingDownstream, EfcptForceCompileDownstreamProjectsBenefits
✅ Works out-of-box: No configuration required for basic scenarios
✅ Always generates models: Even suboptimal configurations produce working builds
✅ Immediate change detection: SQL file edits trigger regeneration without manual DACPAC rebuild
✅ Smart filtering: Only material changes (not formatting) trigger regeneration
✅ Guides optimization: Build logs and README provide clear next steps
✅ Traceability: All JD.Efcpt.Build activity logged for troubleshooting
✅ Non-intrusive: Informational messages instead of warnings
✅ Actionable: Copy-paste-able snippets with correct paths
✅ Valid schema: Sample efcpt-config.json follows proper schema format
✅ Performance: Whitespace-normalized hashing is fast and deterministic
All changes compile successfully and maintain backward compatibility.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.