Skip to content

Conversation

domenkozar
Copy link
Member

@domenkozar domenkozar commented Apr 18, 2025

It's common to be able to build matrix on CI jobs, so let's allow passing options via CLI:

devenv --option services.redis.enable:bool true test

cc @staticdev

@domenkozar domenkozar requested a review from sandydoo April 18, 2025 18:09
@vetsin
Copy link
Contributor

vetsin commented Apr 18, 2025

Given the task model, why not natively support the concept of matrix/parallel within devenv? E.g. in the interest of reproducibility I would want to run a 'matrix task' via devenv not require some sort of shell wrapper to reproduce locally

Copy link

cloudflare-workers-and-pages bot commented Apr 18, 2025

Deploying devenv with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1654ff0
Status:⚡️  Build in progress...

View logs

@domenkozar
Copy link
Member Author

Given the task model, why not natively support the concept of matrix/parallel within devenv? E.g. in the interest of reproducibility I would want to run a 'matrix task' via devenv not require some sort of shell wrapper to reproduce locally

I'd like to support matrix within devenv and has always been the plan, but this is more of a quick way to be able to write scripts using devenv and for testing quick changes.

@vetsin
Copy link
Contributor

vetsin commented Apr 18, 2025

Then i may suggest ENV var support (alternatively/in-addition-to) than building cli arguments -- given that the expectation that it will be 'for CI' it's much easier/the default to set specific variables than deal with command construction imo.

I am also confused on how one may refer to the different package versions if they're not explicitly packaged via nixpkgs -- e.g. the normal way one has multiple versions may require overlays, say:

            (final: prev: {
                go_1_18 = (import (builtins.fetchGit {
                  name = "old-go-1.18";
                  url = "https://github.com/NixOS/nixpkgs/";
                  ref = "refs/heads/nixos-24.05";
                  rev = "20cb53203e8a00ceaa9b1017ced00f23af93a314";
                }) {}).go_1_18;
            })

What would the value of the pkg option be then?

@domenkozar domenkozar force-pushed the cli-options branch 4 times, most recently from f30a95c to 0fdc00b Compare April 21, 2025 10:05
@domenkozar
Copy link
Member Author

I've changed the design to always require specifying the type:

devenv --option env.FOO:bool true

@domenkozar domenkozar marked this pull request as ready for review April 21, 2025 10:06
It requires to specify the infered Nix type:

--option languages.redis.enable:bool true

This is usefor for temporary changing the configuration, like testing a
matrix of options.
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new CLI flag (--option) to allow overriding configuration options for building matrix on CI jobs. Key changes include:

  • Adding a sample YAML file in tests/cli-options for CLI options.
  • Updating documentation in docs/basics.md with a new section on how to use the --option flag.
  • Implementing the logic for parsing and writing CLI options in devenv/src/devenv.rs, along with corresponding updates to CLI parsing and file watching in cnix.rs and cli.rs.

Reviewed Changes

Copilot reviewed 5 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/cli-options/devenv.yaml Added sample configuration inputs for testing CLI options.
docs/basics.md Expanded documentation to include instructions for using the --option flag.
devenv/src/devenv.rs Implemented CLI options processing and writing to cli-options.nix.
devenv/src/cnix.rs Updated file watching to include cli-options.nix.
devenv/src/cli.rs Added new CLI flag definition for --option.
Files not reviewed (3)
  • devenv/src/flake.tmpl.nix: Language not supported
  • tests/cli-options/.test.sh: Language not supported
  • tests/cli-options/devenv.nix: Language not supported
Comments suppressed due to low confidence (1)

devenv/src/devenv.rs:895

  • Using chunks_exact(2) may silently ignore a trailing CLI option if an odd number of values is provided. Consider validating that the total number of --option arguments is even before processing.
for chunk in self.global_options.option.chunks_exact(2) {

@domenkozar domenkozar merged commit 63f24dc into main Apr 21, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants