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

doc: add install test info to hacking.md #6652

Merged
merged 10 commits into from
Oct 5, 2022

Conversation

abathur
Copy link
Member

@abathur abathur commented Jun 11, 2022

@Artturin suggested someone document installer testing (added in #4549, updated in at least #4577) in hacking.md.

I'm a little ambivalent about doing this because it underscores everything else about generating and testing installers that isn't documented--and I don't grok the big picture here. I'm happy to field organization and phrasing nits/clarifications--but fair warning that I will be grumpy if there's pressure to turn this PR into a vehicle to fully document the status quo.

I did stub out a comment with what I can recall of the manual process in case it encourages someone else to flesh that part out, but I'm not sure if it'll cause any trouble in the manual-building pipeline.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/small-installer-idempotence-fix/19791/1

@fricklerhandwerk
Copy link
Contributor

Thank you, it's worthwhile having this documented at all. Yet, it does not feel right to rely on an external service just to do regular development work on Nix itself. I suggest moving this work to NixOS Wiki under the Nix category so you have something more easily readable to link to than an open pull request.

@abathur abathur closed this Aug 4, 2022
@abathur
Copy link
Member Author

abathur commented Aug 4, 2022

I'm tired of shoveling installer-related busywork around while the broader project and community neglect it.

It says a lot that it isn't worth documenting how people can do the bare minimum to ensure they aren't breaking the installer in the contributing section of the manual.

@fricklerhandwerk
Copy link
Contributor

@abathur I want to emphasize that I think it absolutely is worth documenting what you say. @thufschmitt just posted Tweag's roadmap for Nix where we explicitly state that we want to focus on the installer. I transferred your write-up to the NixOS Wiki under Testing the Nix installer.

@abathur
Copy link
Member Author

abathur commented Aug 5, 2022

@fricklerhandwerk I take the point about the external service, but I'm not describing ol' abathur's home-grown installer testin' steps--this is describing the gap people need to close to enable the installer and installer_test jobs already defined in this repository.

I'm flummoxed that this repository isn't the place to document how to use the CI jobs defined in this repository.

@fricklerhandwerk
Copy link
Contributor

but you do realize that I'm [...] describing the gap people need to close to enable the installer and installer_test jobs already defined in this repository, right?

@abathur Truth be told, I did not fully realize it, although it is mentioned in the very description you want to add.

But neither did I close this PR (and would not even be able to), just added my stance on the topic. Sorry for causing you frustration.

@abathur abathur reopened this Aug 5, 2022
Co-authored-by: Valentin Gagarin <[email protected]>
@abathur
Copy link
Member Author

abathur commented Aug 5, 2022

But neither did I close this PR (and would not even be able to), just added my stance on the topic. Sorry for causing you frustration.

Sorry--it looks I'm over-interpreting your decisive mandate re: docs, here, since I know you're working towards those. In that context, it struck me as a decision.

Co-authored-by: Valentin Gagarin <[email protected]>
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/summer-of-nix-documentation-stream/20351/2

Copy link
Contributor

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

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

I was able to follow these instructions successfully and it worked for me. Thanks @abathur!

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

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

Sorry for stalling this for so long. Trying to catch up with GitHub notifications now.

Let's try to make this concise and to the point. Reference material should not tell stories. When we merge this, there will be an obvious starting point for working on the installer, as well as a place for additions and updates to the instructions.

PS: Thanks @Hoverbear for validating the instructions.
PPS: @abathur I already committed minor cosmetic changes so you don't have so much noise to go through.

doc/manual/src/contributing/hacking.md Outdated Show resolved Hide resolved
Comment on lines 114 to 116
Testing the install scripts has traditionally been tedious, but you can now do this much more easily via the GitHub Actions CI runs (at least for platforms that Github Actions supports).

If you've already pushed to a fork of Nix on GitHub before, you may have noticed that the CI workflows in your fork list skipped `installer` and `installer_test` jobs. Once your Nix fork is set up correctly, pushing to it will also run these jobs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Testing the install scripts has traditionally been tedious, but you can now do this much more easily via the GitHub Actions CI runs (at least for platforms that Github Actions supports).
If you've already pushed to a fork of Nix on GitHub before, you may have noticed that the CI workflows in your fork list skipped `installer` and `installer_test` jobs. Once your Nix fork is set up correctly, pushing to it will also run these jobs.
Testing the install scripts is currently most convenient with GitHub Actions.
The Nix repository has [two continuous integration (CI) workflows](https://github.com/NixOS/nix/blob/88a45d6149c0e304f6eb2efcc2d7a4d0d569f8af/.github/workflows/ci.yml#L50-L91), which are run every time changes are pushed to a branch:

Copy link
Member Author

@abathur abathur Sep 17, 2022

Choose a reason for hiding this comment

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

I realize this will grow crusty over time and will be moot if the ~binary installer project reaches fruition, but I'm trying to drop breadcrumbs that invite people who attempted some small installer fix/improvement in the past (only to be scarred by the testing/review ~process) to take a second look.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a reworked draft of the section covered by this edit and the one below, but I'll hold off on committing and pushing until we hash out the title question.

doc/manual/src/contributing/hacking.md Show resolved Hide resolved
doc/manual/src/contributing/hacking.md Outdated Show resolved Hide resolved
doc/manual/src/contributing/hacking.md Outdated Show resolved Hide resolved
doc/manual/src/contributing/hacking.md Outdated Show resolved Hide resolved
doc/manual/src/contributing/hacking.md Outdated Show resolved Hide resolved
doc/manual/src/contributing/hacking.md Outdated Show resolved Hide resolved
Mainly:
- Try to triangulate between narrative that framed this as
  a new/easy process and the need for a reference that will
  not quickly grow stale.
- Fix a ~continuity issue where the text was talking about
  "your Cachix cache" before saying that you'd need to make
  a Cachix cache to enable the installer tests.
- Adopt suggestion on titling, and nest subtitles in the
  installer test section.
@Hoverbear
Copy link
Contributor

It's looking lovely!

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

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

Just a few last nits. Sorry for the noise... perfect is the enemy of done, I know.

If you'd like to make a follow-up at some point in the future, I'd greatly appreciate screenshots to help navigate the GUI steps. It already works as it is, but would be a lot easier with images.

doc/manual/src/contributing/hacking.md Outdated Show resolved Hide resolved
doc/manual/src/contributing/hacking.md Outdated Show resolved Hide resolved
Co-authored-by: Valentin Gagarin <[email protected]>
@fricklerhandwerk fricklerhandwerk merged commit ac0fb38 into NixOS:master Oct 5, 2022
Minion3665 pushed a commit to Minion3665/nix that referenced this pull request Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants