Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flake.nix: use ./tool instead of nixpkgs #14510

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kradalby
Copy link
Member

@kradalby kradalby commented Jan 3, 2025

This ensure consistency between tooling used by nix and non-nix users.

Updates tailscale/corp#25620

Signed-off-by: Kristoffer Dalby kristoffer@tailscale.com

This ensure consistency between tooling used by nix and non-nix users.

Updates tailscale/corp#25620

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
Remove the check if the shell is Nix based, it looks
for a nix store path, which on nix-darwin will not start
with /nix/store (but /etc/profile).

./tool/helm is no used regardless of nix, and nix shell
is using that since prev commit.

Updates #25620

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
Copy link
Contributor

@irbekrm irbekrm left a comment

Choose a reason for hiding this comment

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

./tool/helm is no used regardless of nix

For context, ./too/helm exists so that our release scripts that run on the build server can run helm commands to package helm charts without us needing to install helm CLI there, i.e https://github.com/tailscale/corp/blob/main/misc/build/clients/helmchart.go#L21 (private link)

I think the build servers are still using nix shell? But I might be wrong, I haven't looked at the setup there for a while

@irbekrm
Copy link
Contributor

irbekrm commented Jan 6, 2025

FYI there is also

if [[ -n "${IN_NIX_SHELL:-}" ]]; then

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