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

feat: skip-nix-install #23

Merged

Conversation

mstruebing
Copy link
Contributor

@mstruebing mstruebing commented Sep 27, 2023

We have some problems in our custom runner with this action so I've tried it with skipping the installation via the included action and it works in our runners (we do not use the GitHub provided ones, we are using custom runners).

In case you are wondering which action we are using, we use: cachix/install-nix-action@v23 - I guess it has something to do with the usage of sudo in the other action but I'm not 100% sure on that.
I'm also not sure about the differences between these two actions so I've opted for the possibility to skip the installation rather than changing the action as I guess you've put a lot of thought into which action to use.

Edit: I need to correct myself, it's not because of sudo but because of systemd. I tried to run the action without systemd by passing in init: none and planner: linux-multi but I got other errors. So I would prefer to be able to just skip the installation and use my own / a different action to install nix.

@mstruebing mstruebing force-pushed the mstruebing/skip-nix-installation branch from 4758f98 to 068937a Compare September 27, 2023 09:19
Copy link

@clintonmedbery clintonmedbery left a comment

Choose a reason for hiding this comment

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

Nice work!

Copy link

@savil savil left a comment

Choose a reason for hiding this comment

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

lgtm, thanks! Why drop the step below?

action.yml Outdated
@@ -55,7 +58,6 @@ runs:
path: /usr/local/bin/devbox
key: ${{ runner.os }}-devbox-${{ env.latest_version }}

- name: Install devbox cli
Copy link

Choose a reason for hiding this comment

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

why remove this step?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry good catch, that was a mistake. I readded it.

@savil savil requested a review from LucilleH September 27, 2023 16:20
We have some problems in our custom runner with this action so
I've tried it with skipping the installation via the included action and
it works in our runners (we do not use the GitHub provided ones, we are
using custom runners).

In case you are wondering which action we are using,
we use: `cachix/install-nix-action@v23` - I guess it has something to do
with the usage of `sudo` in the other action but I'm not 100% sure on
that.
@mstruebing mstruebing force-pushed the mstruebing/skip-nix-installation branch from 068937a to 5f7448c Compare September 27, 2023 18:23
Copy link
Contributor

@LucilleH LucilleH left a comment

Choose a reason for hiding this comment

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

LGTM!

@LucilleH LucilleH added this pull request to the merge queue Sep 27, 2023
Merged via the queue into jetify-com:main with commit 6b09349 Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants