lix-project/lix-install-action
1
0
Fork 0
Code Issues 11 Pull requests Packages Projects Releases Wiki Activity

Don't use docker shim if only using a mounted docker.sock instead of docker-in-docker #67

Merged
Hoverbear merged 1 commit from hoverbear/fh-161-after-running-in-act-hosts-nix-daemon-is-unusable into main 2024-01-11 15:55:46 +00:00
Conversation 0 Commits 1 Files changed 3 +203 -2
Hoverbear commented 2024-01-10 19:45:42 +00:00 (Migrated from github.com)
Copy link
Description

I believe this fixes https://github.com/DeterminateSystems/nix-installer-action/issues/65.

Checklist
  • Tested changes against a test repository
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • (If this PR is for a release) Updated README to point to the new tag (leave unchecked if not applicable)
##### Description I believe this fixes https://github.com/DeterminateSystems/nix-installer-action/issues/65. ##### Checklist - [ ] Tested changes against a test repository - [ ] Added or updated relevant documentation (leave unchecked if not applicable) - [ ] (If this PR is for a release) Updated README to point to the new tag (leave unchecked if not applicable)
cole-h (Migrated from github.com) approved these changes 2024-01-10 22:52:11 +00:00
cole-h (Migrated from github.com) left a comment
Copy link

I think this looks sound overall, but I'm not extremely familiar with docker or cgroups and the like, so take this approval with a grain of salt...

I think this looks sound overall, but I'm not extremely familiar with docker or cgroups and the like, so take this approval with a grain of salt...
src/main.ts
@ -185,0 +240,4 @@
stdout: (data: Buffer) => {
stdout_buffer += data.toString("utf-8");
},
stderr: (data: Buffer) => {
cole-h (Migrated from github.com) commented 2024-01-10 22:52:08 +00:00
Copy link

Would it be worth it to check every cgroup line to see if there is /docker/ in it, or if it's in an earlier cgroup, is that a weird configuration we wouldn't want to support anyways?

Would it be worth it to check _every_ cgroup line to see if there is `/docker/` in it, or if it's in an earlier cgroup, is that a weird configuration we wouldn't want to support anyways?
colemickens (Migrated from github.com) approved these changes 2024-01-11 15:28:26 +00:00
colemickens (Migrated from github.com) left a comment
Copy link

Also no cgroup expert, but the code matches what was described, and the description I was given sounded convincing for detecting this scenario and avoiding it.

Also no cgroup expert, but the code matches what was described, and the description I was given sounded convincing for detecting this scenario and avoiding it.
Hoverbear (Migrated from github.com) reviewed 2024-01-11 15:54:27 +00:00
src/main.ts
@ -185,0 +240,4 @@
stdout: (data: Buffer) => {
stdout_buffer += data.toString("utf-8");
},
stderr: (data: Buffer) => {
Hoverbear (Migrated from github.com) commented 2024-01-11 15:54:27 +00:00
Copy link

I don't have access to every system and setup, only a few, but I think in this case we're only looking for the 0:: prefixed line which is always last.

I don't have access to every system and setup, only a few, but I think in this case we're only looking for the `0::` prefixed line which is always last.
Sign in to join this conversation.
Reviewers
No reviewers
cole-h
colemickens
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lix-project/lix-install-action#67
No description provided.
Delete branch "hoverbear/fh-161-after-running-in-act-hosts-nix-daemon-is-unusable"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?