Migrate from flake-utils to flake-parts

[jamesbrink]

Summary

This PR migrates the flake from using flake-utils to flake-parts for better modularity and composability.

Changes

• Replace flake-utils input with flake-parts
• Restructure flake.nix to use flake-parts' module system
• Maintain all existing functionality:
  • All packages (default, comfyui, cuda, Docker images)
  • All apps (run, update, build Docker images)
  • Development shell
  • Checks (nixfmt, ruff, pyright, shellcheck, package build)
  • Formatter
  • NixOS module
  • Overlays
  • Legacy packages (custom nodes)
• Platform-specific constraints preserved:
  • CUDA packages only on x86_64-linux
  • Docker images available for cross-platform builds
  • aarch64-linux workarounds maintained

Benefits

• Better modularity: flake-parts provides a cleaner module system
• Composability: Easier to extend with additional flake-parts modules in the future
• Maintainability: More structured approach to flake organization
• Community alignment: flake-parts is widely adopted in the Nix ecosystem

Testing

• nix flake show - All outputs correctly exposed
• nix flake check - All checks pass
• nix build - Package builds successfully
• Platform constraints verified (CUDA only on x86_64-linux)

Migration Notes

This is a pure refactoring with no functional changes. All existing commands and workflows remain the same.

[claude]

Code Review - Migration from flake-utils to flake-parts

Summary

This PR successfully migrates from flake-utils to flake-parts, which is a well-regarded improvement in the Nix ecosystem. The migration is primarily a refactoring with minimal functional changes.

---

✅ Strengths

1. Clean migration: The transition to flake-parts' module system is well-executed with proper use of perSystem and flake attributes.

2. Functional parity: All existing outputs are preserved:

   • Packages (default, cuda, Docker variants)
   • Apps (run, update, build scripts)
   • Development shells
   • Checks (nixfmt, ruff, pyright, shellcheck)
   • NixOS module and overlays

3. Correct use of flake-parts patterns:

   • Proper perSystem for system-specific outputs
   • flake attribute for system-independent outputs (lib, overlays, modules)
   • Use of self' for same-system references

4. Platform constraints maintained: CUDA packages correctly limited to x86_64-linux, Docker cross-compilation preserved.

5. Improved modularity: The explicit systems list and structured module approach make the flake more maintainable.

---

🔍 Observations & Minor Considerations

1. Redundant pkgs configuration (flake.nix:152-161)

The PR correctly overrides _module.args.pkgs to configure nixpkgs with specific settings. However, there's still manual creation of pkgsLinuxX86 and pkgsLinuxArm64 in the let-block above. This is intentional for cross-platform Docker builds but could be documented better.

Suggestion: Add a comment explaining why these separate pkgs instances exist despite the _module.args.pkgs override:

# Manual pkgs instances for cross-platform Docker builds
# Cannot use _module.args.pkgs here as we need specific target systems
pkgsLinuxX86 = import nixpkgs { ... };

2. Unused function parameters (flake.nix:41-47)

The perSystem function receives config, inputs', and lib parameters that don't appear to be used in the current implementation. This is fine - flake-parts provides them for consistency - but worth noting.

3. apps.nix signature change

The change from:

# Before
apps = import ./nix/apps.nix { inherit pkgs packages; };
# After  
apps = import ./nix/apps.nix { inherit pkgs; packages = self'.packages; };

This is correct and leverages flake-parts' self' reference. Well done.

4. checks.nix signature change

Similarly updated to use self'.packages:

checks = import ./nix/checks.nix {
  inherit pkgs source;
  packages = self'.packages;
  pythonRuntime = nativePackages.pythonRuntime;
};

This is the proper pattern for flake-parts.

---

🔒 Security & Performance

• No security concerns: The migration doesn't introduce any new security risks.
• No performance impact: This is purely a build-time refactoring; runtime behavior is unchanged.
• Dependency update: flake-parts is actively maintained by hercules-ci and widely trusted in the Nix ecosystem.

---

📋 Testing Verification

The PR description indicates testing was performed:

• ✅ nix flake show - All outputs correctly exposed
• ✅ nix flake check - All checks pass
• ✅ nix build - Package builds successfully
• ✅ Platform constraints verified

Recommendation: Once CI checks pass, this should be safe to merge.

---

📝 Suggestions (Optional Improvements)

1. Documentation: Consider adding a brief migration note in CLAUDE.md or a changelog entry explaining why this change was made for future reference.

2. Comment clarity: Add inline comments explaining the cross-platform pkgs instances to help future maintainers understand the architecture.

---

✨ Conclusion

LGTM (Looks Good To Me) with minor suggestions for documentation improvements.

This is a solid refactoring that aligns with Nix community best practices. The migration to flake-parts will make future enhancements easier and improve composability with other flake-parts modules. The implementation correctly preserves all functionality while improving code organization.

Recommendation: Approve and merge once CI passes.

---

🎯 Overall Assessment

• Code Quality: ⭐⭐⭐⭐⭐ Excellent
• Best Practices: ⭐⭐⭐⭐⭐ Follows flake-parts patterns correctly
• Potential Bugs: ✅ None identified
• Security: ✅ No concerns
• Test Coverage: ⭐⭐⭐⭐ Good (could add explicit flake-parts integration tests)

[claude]

Code Review: Migration from flake-utils to flake-parts

I've reviewed this PR and overall it's a well-executed refactoring that improves the flake's modularity and maintainability. Here's my detailed feedback:

✅ Strengths

1. Clean Migration: The transition from flake-utils.lib.eachDefaultSystem to flake-parts.lib.mkFlake is done correctly, with proper use of perSystem and flake sections.

2. Preserved Functionality: All existing outputs are maintained:

   • Packages (default, comfyui, cuda, Docker images)
   • Apps (run, update, build Docker)
   • Dev shells, checks, formatter
   • NixOS module and overlays

3. Better Structure: The separation between per-system (perSystem) and system-agnostic (flake) outputs is cleaner and more idiomatic.

4. Excellent Documentation:

   • Inline comments explain key decisions (e.g., why separate pkgs instances are needed)
   • CHANGELOG.md is comprehensive and follows Keep a Changelog format
   • Parameter comments in perSystem are helpful

5. Correct Use of flake-parts Features:

   • self'.packages for same-system references (nix/apps.nix:193)
   • _module.args.pkgs for configuring the pkgs instance
   • Explicit systems list instead of defaultSystems

🔍 Issues Found

1. Interface Change in nix/apps.nix (IMPORTANT)

Location: nix/apps.nix:2-3

The interface changed from:

{ pkgs, packages }

to:

{ pkgs, packages }  # but packages = self'.packages instead of plain packages

While this works, it's an implicit breaking change. The apps module now expects self'.packages but this isn't documented.

Impact: Low (internal module, but could break if someone uses this as a template)

Recommendation: Either:

• Add a comment in nix/apps.nix documenting this requirement
• Or make the interface more explicit with a better parameter name

2. Similar Issue in nix/checks.nix

Location: nix/checks.nix:4

Changed from packages to packages = self'.packages. Same concern as above.

3. CHANGELOG Version Mismatch

Location: CHANGELOG.md:8-11

The changelog shows this as [Unreleased] but the PR is being merged into main. Should this be versioned as a patch release (e.g., v0.7.0-2) or will it be released as part of the next minor version?

Recommendation: Clarify the release strategy before merging.

4. Potential Redundant pkgs Configuration

Location: flake.nix:158-166

The _module.args.pkgs override duplicates the allowUnsupportedSystem logic that already exists for pkgsLinuxArm64. This is technically correct (native aarch64-linux builds need it too), but the comment could be clearer.

Existing:

# aarch64-linux needs this workaround for kornia-rs
allowUnsupportedSystem = system == "aarch64-linux";

Better:

# aarch64-linux needs this workaround for kornia-rs badPlatforms issue
# This applies to both native builds and cross-compilation targets
allowUnsupportedSystem = system == "aarch64-linux";

💡 Suggestions

1. Consider nixpkgs.lib for lib parameter (Optional)
   Instead of lib = pkgs.lib;, you could use the nixpkgs-lib input from flake-parts for a lighter dependency when building the flake lib (flake.nix:242-243). However, this is a minor optimization.

2. Explicit Systems Documentation (Nice-to-have)
   Add a comment above the systems array explaining why these specific systems are supported:

   # Supported systems: Linux (x86_64, aarch64), macOS (Intel, Apple Silicon)
   # Note: CUDA support is only available on x86_64-linux
   systems = [ ... ];

3. Test Coverage (Question)
   The PR description mentions testing with nix flake show and nix build, but did you test:

   • nix run on all platforms?
   • nix develop?
   • Docker image builds on macOS (cross-compilation)?
   • The NixOS module integration?

🔒 Security & Performance

• No security concerns: The refactoring is mechanical and doesn't introduce new attack vectors
• No performance impact: flake-parts is a build-time dependency only
• Build reproducibility: Maintained through proper input pinning in flake.lock

📝 Code Quality

• Nix style: Follows nixpkgs-rfc-style formatting ✅
• Comments: Well-documented ✅
• Complexity: Appropriate - no over-engineering ✅
• Backwards compatibility: Fully maintained for end users ✅

🎯 Recommendation

APPROVE with minor suggestions

This is a solid refactoring that modernizes the flake architecture. The migration is correct and well-documented. The only real concern is the undocumented interface change in nix/apps.nix and nix/checks.nix, but since these are internal modules, it's not a breaking change for users.

Before merging, please:

1. Consider adding comments about the self'.packages requirement in nix/apps.nix
2. Clarify the CHANGELOG versioning strategy
3. Confirm all apps/dev shells were tested on at least one platform

Great work on this migration! 🎉

[claude]

Code Review: Migration from flake-utils to flake-parts

This PR successfully migrates the flake from flake-utils to flake-parts. The migration is well-executed and follows Nix ecosystem best practices. Here's my detailed review:

---

✅ Strengths

1. Complete Migration: The migration from flake-utils to flake-parts is thorough and properly structured:

   • Correctly uses perSystem for per-system outputs (packages, apps, devShells, checks)
   • Properly uses flake attribute for system-independent outputs (lib, overlays, nixosModules)
   • All existing functionality preserved

2. Excellent Documentation: The flake now includes extensive inline comments:

   • Clear explanation of CUDA support strategy (lines 52-61)
   • Detailed reasoning for separate pkgs instances for cross-platform Docker builds (lines 63-66)
   • System support documentation with CUDA availability note (lines 32-33)
   • Well-documented module parameter bindings (lines 43-48)

3. Proper Use of flake-parts Features:

   • _module.args.pkgs configuration (lines 160-167) ensures consistent nixpkgs settings
   • self'.packages reference in apps.nix (line 195) uses the flake-parts convention
   • Clean separation of concerns between per-system and flake-level outputs

4. CHANGELOG Addition: Great to see a comprehensive CHANGELOG.md following Keep a Changelog format. This will greatly help users track changes.

5. Backwards Compatibility: All existing outputs remain available with the same interface:

   • Same package names
   • Same app names
   • Same overlay structure
   • Same NixOS module interface

---

📝 Observations & Minor Suggestions

1. Version Numbering: Using 0.7.0-2 for a flake refactoring is a bit unconventional. Since this is a pure refactoring with no functional changes, you might consider:

   • Not creating a new version tag at all (just document in changelog)
   • Or using a fourth version segment: 0.7.0.1 or 0.7.0+1
   • Keep in mind that semantic versioning typically uses pre-release identifiers differently

2. nix/apps.nix Signature Change: The apps.nix signature changed from { pkgs, packages } to { pkgs; packages = self'.packages; }. This is correct, but worth noting that:

   • The new approach using self'.packages is more idiomatic for flake-parts
   • The CUDA-specific apps still use conditional attribute checking which works well

3. nix/checks.nix Signature: Similar signature change from { pkgs, source, packages, pythonRuntime } - all parameters are now explicitly passed. This is good practice.

4. Cross-platform Docker Builds: The separate pkgsLinuxX86 and pkgsLinuxArm64 instances are well-justified and necessary for cross-compilation. The documentation explaining this (lines 63-66) is excellent.

---

🔍 Potential Concerns

1. No Functional Test Evidence: While the PR description mentions testing, CI is currently pending. Consider:

   • Waiting for CI to complete before merging
   • Verifying that nix flake check passes on all platforms
   • Testing cross-platform Docker builds if possible

2. Breaking Changes Risk: Although this should be a pure refactoring, there's always risk with major structural changes:

   • ✅ Output structure appears preserved
   • ✅ All packages/apps/checks exposed in same way
   • ⚠️ Internal consumers using inputs.comfyui-nix.lib should verify it still works

3. flake.lock Changes: The flake.lock shows:

   • flake-utils removed
   • flake-parts added with nixpkgs-lib dependency
   • This is expected and looks correct

---

🎯 Best Practices Validation

✅ Nix Code Style: Properly formatted with nixfmt-rfc-style
✅ Module Structure: Clean separation of perSystem vs flake outputs
✅ Documentation: Excellent inline comments
✅ Versioning: flake.lock properly updated
✅ Backwards Compatibility: All existing interfaces preserved

---

🚀 Recommendation

Approve with minor suggestions:

This is a high-quality refactoring that improves the project's maintainability and aligns with modern Nix ecosystem practices. The migration to flake-parts is well-executed and properly documented.

Before merging:

1. ✅ Wait for CI checks to complete
2. ✅ Consider the version numbering feedback (optional)
3. ✅ Test on at least one platform to verify functionality

After merging:

• Consider adding a note to the README about the flake-parts migration for transparency
• Monitor for any user reports of breakage (unlikely given the thorough approach)

---

📚 References

• flake-parts documentation
• Keep a Changelog
• Semantic Versioning

Great work on this migration! 🎉