[BE-1876] Integrate nixpkgs plugin #1156
Conversation
SummaryIntegrated nixpkgs plugin support for faster Ruby installations in mise tool provider. Added backend-prefixed tool request handling, conditional nixpkgs backend usage based on BITRISE_TOOLSETUP_FAST_INSTALL environment variable, and updated version resolution to work with backend-prefixed tool names. Walkthrough
|
![bitrise[bot]](https://avatars.githubusercontent.com/in/32878?s=60&v=4){kind=small}
ofalvai
force-pushed
the
BE-1876-integrate-nixpkgs-plugin
branch
from
661ecec to
a7e38f7
Compare
ofalvai
force-pushed
the
BE-1876-integrate-nixpkgs-plugin
branch
7 times, most recently
from
0b3c71e to
79f7d2a
Compare
integrationtests/toolprovider/mise/install_nixpkgs_ruby_test.go
Outdated
Show resolved
Hide resolved
ofalvai
force-pushed
the
BE-1876-integrate-nixpkgs-plugin
branch
from
79f7d2a to
9db8c89
Compare
- Do not clone + link the plugin, use the `mise plugin install ID URL` command instead - Move as much nix plugin specific logic to install.go as possible
MiseExecutor as an interface was created for testing, but I relized this is redundant and ExecEnv should be mockable instead.
This is what we must use a prefix in tool install commands, so let's use the final value (which can be different than what is the git repo's name)
ofalvai
force-pushed
the
BE-1876-integrate-nixpkgs-plugin
branch
2 times, most recently
from
037d82f to
6bf3ff2
Compare
Also don't return unexpected errors from the function. It's not that the caller would do anything with unexpected errors, it's just going to fallback to the default behavior
ofalvai
force-pushed
the
BE-1876-integrate-nixpkgs-plugin
branch
from
6bf3ff2 to
59c8133
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR integrates a nixpkgs plugin for mise to enable faster tool installation for Ruby, with fallback to the core plugin. The changes refactor the mise execution environment into an interface to support testing, add logic to conditionally use the nixpkgs backend, and temporarily force edge stack detection for integration testing.
Key changes:
- Refactored
ExecEnvfrom a struct to an interface withMiseExecEnvimplementation to support testing and dependency injection - Added nixpkgs plugin integration with conditional logic to fall back to core plugins when nixpkgs is unavailable or versions don't match
- Temporarily hardcoded
isEdgeStack()to return true for testing purposes
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| toolprovider/mise/execenv/execenv.go | Converted ExecEnv to interface with MiseExecEnv implementation, exposing InstallDir as a method |
| toolprovider/mise/helpers_test.go | Added fakeExecEnv test helper implementing ExecEnv interface (replaces mockMiseExecutor) |
| toolprovider/mise/install.go | Added nixpkgs integration logic with canBeInstalledWithNix and installRequest functions |
| toolprovider/mise/install_plugin.go | Updated log statement prefix to [TOOLPROVIDER] |
| toolprovider/mise/install_test.go | Added tests for nixpkgs integration and updated existing tests to use fakeExecEnv |
| toolprovider/mise/mise.go | Updated to use new MiseExecEnv constructor, temporarily hardcoded isEdgeStack to return true |
| toolprovider/mise/nixpkgs/nixpkgs.go | New package with nixpkgs backend decision logic and nix availability check |
| toolprovider/mise/resolve.go | Updated to use ExecEnv interface, removed MiseExecutor interface, renamed variables for clarity |
| toolprovider/mise/resolve_test.go | Refactored tests to use fakeExecEnv instead of mockMiseExecutor |
| toolprovider/asdf/install_plugin.go | Updated log statement prefix to [TOOLPROVIDER] |
| integrationtests/toolprovider/mise/install_nixpkgs_ruby_test.go | Added integration tests for nixpkgs Ruby installation |
| bitrise.yml | Temporarily updated to use edge stack for integration tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
integrationtests/toolprovider/mise/install_nixpkgs_ruby_test.go
Outdated
Show resolved
Hide resolved
|
|
||
| useNix, err := nixpkgs.ShouldUseBackend(tool) | ||
| if err != nil { | ||
| // Note: if Nix is unavailable we cannot force install. |
There was a problem hiding this comment.
See my comment above about returning an error from ShouldUseBackend(). I don't think this whole block is necessary if we run CI on edge stacks temporarily.
| _, err = execEnv.RunMisePlugin("install", nixpkgs.PluginName, nixpkgs.PluginGitURL) | ||
| if err != nil { | ||
| log.Warnf("Error while installing nixpkgs plugin (%s): %v. Falling back to core plugin installation.", nixpkgs.PluginGitURL, err) | ||
| // Warning, if false is not returned here, force install will be allowed even though plugin install failed. |
There was a problem hiding this comment.
I don't get this part, what is a force install?
There was a problem hiding this comment.
I think you added it here to make integration tests possible.
| return false | ||
| } | ||
|
|
||
| _, err = execEnv.RunMisePlugin("update", nixpkgs.PluginName) |
There was a problem hiding this comment.
Why do we need this update call if we just installed the plugin right above?
There was a problem hiding this comment.
Since it's crucial for the plugin to be up-to-date I wanted to make sure it's on the latest commit, even if it becomes preinstalled at some point. There are two options for this: install with --force flag (reinstall plugin), or call update. I chose update because It seems less dangerous.
| } | ||
| log.Debugf("Mise: Stack is edge: %s", isEdge) | ||
| return | ||
| // if stack, variablePresent := os.LookupEnv("BITRISEIO_STACK_ID"); variablePresent && strings.Contains(stack, "edge") { |
There was a problem hiding this comment.
I commented this out because this doesn't catch the Linux edge stack, so we install a too old Mise version.
We could bump our pinned stable Mise version, the unstable Mise version got enough testing on macOS edge stacks I think.
Checklist
README.mdis updated with the changes (if needed)Version
Requires a MAJOR/MINOR/PATCH version update
Context
Changes
Investigation details
Decisions