devenv: support containers on macOS

[sandydoo]

This unlocks the container restriction on macOS. You will need an <arch>-linux builder already set up in your nix.conf to build the container.

The underlying flake now exports configurations for all 4 systems.

Fixes #430

[github-actions]

🔍 Suggested Reviewers

Based on git blame analysis of the changed lines, the following contributors have significant experience with the modified code:

• @domenkozar - 100.0% of changed lines (123 lines)

Please consider reviewing this PR as you have authored significant portions of the code being modified. Your expertise would be valuable! 🙏

This comment was automatically generated by git-blame-auto-reviewer

Last updated: 2025-09-12T11:52:13.720Z

[claude]

Claude finished @sandydoo's task —— View job

---

PR Review: devenv containers on macOS

I've analyzed the code changes in this PR. Here are the key issues and improvements to consider:

🔧 Code Quality & Architecture Issues

1. Hard-coded architecture mapping (devenv.rs:479-487)

   let target_system = if host_os == "macos" {
       match host_arch {
           "aarch64" => "aarch64-linux",
           "x86_64" => "x86_64-linux",
           _ => bail!("Unsupported container architecture for macOS: {host_arch}"),
       }
   } else {
       &self.global_options.system
   };

   • This hard-coded mapping is fragile and doesn't handle future architectures
   • Consider using a configuration-driven approach or helper function
   • The error message could be more helpful by listing supported architectures

2. Inconsistent error handling

   • Line 483: Uses bail! macro for architecture errors
   • Consider providing guidance on how to add cross-platform builder support

⚠️ Potential Bugs

3. Missing validation in flake template

   • The new perSystem structure generates configurations for all systems but doesn't validate if required builders exist
   • Consider adding runtime checks for cross-platform builds on macOS

4. Unsafe environment variable usage (build.rs:4,8)

   std::env::var("CARGO_CFG_TARGET_ARCH").unwrap()
   std::env::var("CARGO_CFG_TARGET_OS").unwrap()

   • Using .unwrap() on environment variables that should always exist in build context
   • While these are guaranteed by Cargo, consider more descriptive error messages

🚀 Performance Considerations

5. Redundant system evaluation

   • The flake template now generates all 4 system configurations even when only one is needed
   • For container builds, this increases evaluation time unnecessarily
   • Consider lazy evaluation or system-specific builds

6. Memory usage in flake template

   • Lines 161-176: All system configurations are kept in memory simultaneously
   • For large projects, this could increase memory consumption significantly

🔒 Security Concerns

7. Cross-compilation trust boundary
   • Building Linux containers on macOS requires remote builders, expanding attack surface
   • No validation that the remote builder is configured securely
   • Consider documenting security best practices for cross-platform builds

📋 Test Coverage Issues

8. Limited test coverage for new functionality

   • New test only validates basic build output parsing
   • Missing tests for:
     • Cross-architecture container builds
     • Error handling for missing builders
     • macOS-specific code paths
   • Consider adding integration tests for the container build flow

9. Test configuration lacks macOS scenarios

   • tests/outputs/.test.sh doesn't test macOS-specific behavior
   • No tests for architecture detection and mapping logic

💡 Suggested Improvements

10. Architecture mapping helper

    fn map_host_to_container_arch(host_os: &str, host_arch: &str) -> Result<&str> {
        match (host_os, host_arch) {
            ("macos", "aarch64") => Ok("aarch64-linux"),
            ("macos", "x86_64") => Ok("x86_64-linux"), 
            ("macos", arch) => bail!("Unsupported macOS architecture '{}'. Supported: aarch64, x86_64", arch),
            _ => Ok(host_arch), // Use system default for non-macOS
        }
    }

11. Builder validation

    • Add checks to verify required builders are configured before attempting cross-platform builds
    • Provide helpful error messages with setup instructions

12. Documentation

    • The PR description mentions needing <arch>-linux builder setup but this isn't validated at runtime
    • Consider adding runtime checks and helpful error messages

✅ Todo Summary

• Analyze the main Nix template changes in flake.tmpl.nix
• Review the new build.rs file
• Check devenv.rs modifications
• Examine test configuration changes
• Look for potential issues and improvements
• Provide comprehensive feedback

The core functionality looks solid, but the implementation could be more robust with better error handling, validation, and test coverage for the cross-platform scenarios.

---

[cloudflare-workers-and-pages]

Deploying devenv with    Cloudflare Pages

Latest commit:	ede4c48
Status:	🚫  Build failed.

View logs

[domenkozar]

Nice :) Let's merge, needs some rebasing :)

[sandydoo]

After the TUI work, I think we should always pass commands through CacheCommand. Not for caching, but to get access to the parsed logs and capture the errors. This would let us detect the missing builder error and suggest ways to set one up.

[domenkozar]

@sandydoo let's merge this?