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.

Sign up for GitHub

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

Jump to bottom

nixos/rss2email: move from /var to /var/lib #85317

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

nixos/rss2email: move from /var to /var/lib #85317

wants to merge 1 commit into from
+10 −7

Conversation

alyssais
Copy link
Member

This way, we can use StateDirectory instead of a tmpfile rule.

@alyssais
nixos/rss2email: move from /var to /var/lib

Verified

This commit was signed with the committer’s verified signature.
MichaReiser Micha Reiser
SSH Key Fingerprint: ecGsN1ElBc6rDn4Ge3zV90y0fffKRH+1PNbJCxfpPEs
Verified
Learn about vigilant mode
a235ef6 
This way, we can use StateDirectory instead of a tmpfile rule.
@alyssais alyssais requested a review from Ekleog April 15, 2020 17:48
@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Apr 15, 2020
@Ekleog
Copy link
Member

Ekleog commented Apr 15, 2020

This looks good :) that said… it means that it's not possible to move back from /var/lib to /var by just switching to the previous generation, and thus I think it should be gated on nixos.stateVersion? Either that or leave around a symlink from /var to /var/lib upon migration, maybe?

@flokli
Copy link
Contributor

flokli commented Apr 16, 2020

I think @aanderse had some plans on moving more state to native StateDirectory locations, so this might be a good example.

Mic92
Mic92 reviewed Apr 16, 2020
View reviewed changes
nixos/modules/services/mail/rss2email.nix
];
system.activationScripts.rss2email = lib.stringAfter [ "users" ] ''
if [ -e /var/rss2email -a ! -e /var/lib/rss2email ]; then
mv /var/rss2email /var/lib/rss2email
Copy link
Member

Choose a reason for hiding this comment

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

I suppose this script can be optionally included depending on state version?
The tmpfiles rules line above needs to be changed to /var/lib/rss2mail because new users won't have this directory yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this should be an optionalString and gated on stateVersion. I'd prefer having it inside preStart too.

@alyssais
Copy link
Member Author

alyssais commented Apr 16, 2020 via email

@alyssais
Copy link
Member Author

alyssais commented Apr 16, 2020 via email

@Ekleog
Copy link
Member

Ekleog commented Apr 16, 2020

Do we make unstable installs default to 20.09? That sounds broken, I'd have expected them to start with a stateVersion of 20.03. IMO either linking from /var to /var/lib in the migration script or gating on 20.09 are good -- AFAIK we always use the next release's stateVersion when doing backwards-incompatible changes

@flokli
Copy link
Contributor

flokli commented Jun 14, 2020

Do we make unstable installs default to 20.09? That sounds broken, I'd have expected them to start with a stateVersion of 20.03. IMO either linking from /var to /var/lib in the migration script or gating on 20.09 are good -- AFAIK we always use the next release's stateVersion when doing backwards-incompatible changes

Yeah, we should probably only update the stateVersion rendered by nixos-generate-config immediately before the release, but this is probably a discussion that should be done outside the release.

I'd also be fine with not having it conditional on stateVersion, but then it should really live in preStart, and add a comment linking to this issue, and stating it's for migration of that file.

@stale
Copy link

stale bot commented Dec 12, 2020

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 12, 2020
@flokli
Copy link
Contributor

flokli commented Dec 12, 2020

@alyssais can you gate this on stateVersion and move it to the preStart script?

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 12, 2020
@flokli flokli added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 12, 2020
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 20, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank marked this pull request as draft March 25, 2024 16:19
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92

@flokli flokli

@aanderse aanderse

@Ekleog Ekleog

Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@alyssais @Ekleog @flokli @Mic92 @aanderse @wegank