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

Test macos/linux installer script for each push #4549

Merged
merged 2 commits into from
Feb 25, 2021
Merged

Conversation

domenkozar
Copy link
Member

@domenkozar domenkozar commented Feb 15, 2021

This works by using Cachix feature of serving a file from
a store path.

Fixes #4564
Fixes #4047

cc @edolstra

@edolstra
Copy link
Member

Wouldn't it be better to serve the tarball from GitHub as an uploaded artifact?

It doesn't yet works because it seems like that flake shim sets self.shortRev for checks attribute but not for outputs?

This is probably because changing flake.nix dirties the tree, so there is no shortRev.

@domenkozar
Copy link
Member Author

Wouldn't it be better to serve the tarball from GitHub as an uploaded artifact?

Yes! However that's really tricky to implement as the installer script wants urls hardcoded for each platform.

This is probably because changing flake.nix dirties the tree, so there is no shortRev.

What's the workaround here? A new flake that changes input for systems? Some way to pass in --arg?

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

You are missing final new lines.

@domenkozar
Copy link
Member Author

@abathur this PR creates an artifact, that is a zip file containing a tar that has the executable to install Nix.

That's a huge hack due to limitations of GHA that it always creates a zip out of artifacts and that those artifacts are never executables.

Maybe it would be easier to just print the url for the installer instead, that would allow us to use it with cachix-action.

WDYT?

@domenkozar domenkozar force-pushed the installer-artifact branch 7 times, most recently from 08d19d9 to d8daada Compare February 21, 2021 13:27
This works by using Cachix feature of serving a file from
a store path.
@domenkozar domenkozar changed the title Generate installer script for each PR/push Test macos/linux installer script for each push Feb 21, 2021
@domenkozar
Copy link
Member Author

This PR now adds automated testing of installer script via GHA.

  • builds installer script for linux/macos and pushes it to Cachix
  • uses install-nix-action by pointing to Cachix (it has a non-advertised feature to serve files from nars)

@domenkozar
Copy link
Member Author

@edolstra this is ready to be merged and it already caught a bug :)

Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

Really cool!

Is there a way to print some instructions for installing this locally? Afaict the actuall installer URL is never shown in the logs nor exported in any way (maybe this could even be exported in an artifact)

@domenkozar
Copy link
Member Author

I tried an artifact before, but it doesn't preserve execution bit.

install-nix-action inputs are printed in beginning of the log

@thufschmitt
Copy link
Member

I tried an artifact before, but it doesn't preserve execution bit.

Yeah I saw that, I was just thinking of a plain text file containing the actual URL or something like that. But it's not really important, and certainly not a blocker for this PR.

install-nix-action inputs are printed in beginning of the log

Uh. So today I learned that the little triangle at the begining of the Run line isn't some kind of shell prompt but a button to display more infos about what's being run 🤦‍♂️

@edolstra edolstra merged commit c189031 into master Feb 25, 2021
@edolstra edolstra deleted the installer-artifact branch February 25, 2021 15:08
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/anyone-up-for-picking-at-some-nix-onboarding-improvements/13152/1

#- run: nix flake check
- run: nix-build -A checks.$(if [[ `uname` = Linux ]]; then echo x86_64-linux; else echo x86_64-darwin; fi)
installer:
if: github.event_name == 'push'
Copy link
Member

Choose a reason for hiding this comment

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

The commit message says PR/push while this is only on push

would this test have caught the typo introduced in #6285 if the test was run?

Copy link
Member

Choose a reason for hiding this comment

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

Probably, though iirc they still won't run if you don't have your own cachix key (and, as a glance at the master history shows, these are not always green).

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.

Create a working installer script for each commit/PR
7 participants