Skip to content

Fail faster on missing deps #52

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

Closed
wants to merge 2 commits into from
Closed

Conversation

KeyWeeUsr
Copy link

When running locally with act this fails depending on the image used because it relies on some, binaries being available by default:

  • curl probably won't be as it's common to have wget or neither
  • xz might not be present in case "complete" tar installation is provided
  • and, if working with Alpine, sudo either won't be because doas is recommended there or the runner image is custom-built without super-user capabilities in mind

This notifies the user faster because normally it'll fail on sudo first, then after the first iteration it'll fail on curl and after the second on missing xz for decompression.

@KeyWeeUsr
Copy link
Author

@purcell Ping

Copy link
Owner

@purcell purcell left a comment

Choose a reason for hiding this comment

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

I'm a bit wary of trying to capture the nix installer script's deps in order to just check them slightly earlier, particularly xz, which isn't even mentioned there. Presumably even without these checks, it's possible to see what isn't installed. Also with your PR, given no preinstalled binaries, one would still need to run the action three times and fix the missing dependency each time, so I think you'd need to do something like my suggestion above.

But in any case, I don't feel this change is likely to be worth the trouble, sorry.

Co-authored-by: Steve Purcell <[email protected]>
@KeyWeeUsr
Copy link
Author

I included the suggestion and also have given it a second thought. This way it's prone for more and more missing deps depending on the environment incompleteness. Just for this one it's already 3 with a default image act uses. Failing faster would be much better than downloading and crashing, but it's only a "hack" and I agree.

Instead, Nix should be doing this 1) during installation if this is a common toolset used 2) prior to executing a beefy part if this is Emacs-installer-specific.

I'll leave it up to you whether to include this patch or not 👍

@purcell
Copy link
Owner

purcell commented Jul 9, 2025

Yeah, thanks, I think I'll skip merging this here. Upstream would indeed be better. Cheers!

@purcell purcell closed this Jul 9, 2025
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