-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Users upgrading gh
to v2.40.0 may see failures to write the config file
#4744
Comments
From what I can tell, older versions of |
@msfjarvis ahhh this is possible but might have some unintended consequences. I'm not totally sure whether its intended that your The data migration that occurs to support multiple users will only run if the version is not present. So if the version key is in there for an older version of Using
Log in to an account
See that the hosts file is in the old format:
Write the version to the config file to do what your suggestion would do:
Now log in with
And we can see that the
This occurred because This isn't really the end of the world, it just means that the user would need to log in to the first account again but you know, it could confuse people that expected to have a new account added since they got the new version. It's probably not confusing than any other option though 😅 |
Hm yeah that's definitely a problem. I'll give this some more thought and see if a better solution reveals itself to me. |
Is there any way for There's some bad yaml formatting occurring in there by |
Is that a problem? The It might be pretty hard to write the migration though without extra tooling that can work with yaml. |
Hm yeah that's true, I'll give it a shot.
Nixpkgs has the ability to at least generate YAML, I'll have to check if we can also parse it. |
For people that are coming here because they encounter an error such as:
then a workaround I've found is that I could configure programs.gh = {
enable = true;
settings = {
# Workaround for https://github.com/nix-community/home-manager/issues/4744
version = 1;
...
};
}; The migration of |
@Tenzer thank you for chiming in. That will indeed work and I don't think it will give you any more issues moving forwards. @msfjarvis I thought a bit more about writing a migration and I think maybe it's not the best idea because we might end up diverged at some point. It just seems better to hash out a solution that works generally going forward. I'll speak with the team about it today as well. |
I agree. I'm currently out on vacation so it's been harder to find time to iterate on this, but a potential solution I thought of is to introduce a |
Hi, thanks for the heads up, I looked into it and don't really see too many options. Then we can add the correct version string based on |
In this case the ideal would be to make the module keep using the old version of gh for That said, it seems like the old version of gh does not exist in Nixpkgs so I guess it needs some alternative solution. To this end. Is it possible to detect whether the migration has happened using grep, yq, or similar? If so, then you could add a check step in the activation script. Something like: home.activation.checkGhV1Migration = hm.dag.entryBefore ["writeBoundary"] ''
if grep -q "magic stuff" "$HOME/whatever"; then
_iError $'The existing gh state files are not compatible with gh version 2.40.0.
To migrate the state please run the command suitable for your installation method:
Channel: nix-shell <nixpkgs> -p …
Flake: nix run …'
exit 1
fi
''; It is a bit drastic, perhaps, but at least it'll be clear for the user what is going on. |
Is the
Edit: actually, the error message shows up, but the account itself is switched after the operation. I still think it's a weird problem that this error shows up in the first place when the operation completes successfully though. |
Yes, in that writing the
Switching account should not result in writing the All this said, I have to be honest that I did not expect this to be so problematic for the I've also mentioned over on the |
Looking through the comments again it seems like it wouldn't be too tricky to do the migration as part of the activation script (as suggested by @Janik-Haag. Something like home.activation.migrateGhAccounts =
hm.dag.entryBetween [ "linkGeneration" ] [ "writeBoundary" ] ''
# Check if migrated and, if not, run some command that does the migration.
''; That should make the migration run after the activation script is allowed to mutate the user environment but before the updated settings (which would include Since Nixpkgs unstable only has 2.40.0+ available I think we can do this migration unconditionally and not worry about looking at the package |
Any command will attempt to run the migration, so I would expect something like this to work, assuming that
This will search for the string I'm not sure all the environments in which That said, the first thing that I can't speak to the package |
Ok, cool. I've updated the #4749 PR to include this activation script thingy. It will run |
I've created an issue there. Thanks! |
Update all inputs, and temporarily resolve 'gh' migration issues by replacing the 'github:nix-community/home-manager' branch with 'github:msfjarvis/home-manager/hs/gh-2.40-compat'. Additionally, remove the now redundant 'beautysh' input. Related: nix-community/home-manager#4744 Requires: nix-community/home-manager#4749
Amazing, thank you @msfjarvis and @rycee for the fix and everyone else for their involvement in this issue. |
Description
In
gh
v2.40.0, we are going to be releasing initial support for multiple accounts on a single host. As part of this process there is a migration of on-disk data and we will be adding aversion
field with value1
to theconfig.yml
file.I'm not too familiar with
nix
or how people use it, so I'm not sure if the following is even an expected workflow. If it is expected that you can update the version ofgh
in place without regenerating theconfig.yml
file fromhome-manager
scripts it is likely that users will run into an error whengh
tries to write theconfig.yml
file.Although it's our position that
gh
manages its own configuration file, we also try not to break users that choose to configure their systems this way. Unfortunately, in this case we value the future maintainability of having thisversion
to support future migrations should we need them.As such, this is just an informative issue for people to find that this new field will be required to perform any operations with v2.40.0. Since I'm not totally sure on the workflows used in this project, here are some possible workarounds:
config.yml
one time so that thisversion
can be writtenhome-manager
scripts withversion: 1
declared in the confighome-manager
scripts withversion: 1
declared in the config and then blow everything away and start with v2.40.0 from scratchTo me it feels like option
3
is the most sensible, since home manager enforces config to be declaratively defined up front, the only consequence will be requiringgh auth login
to be run for any accounts that were removed when blowing it all away. However, I will again point to my ignorance in this space 😅We apologise for the inconvenience.
The text was updated successfully, but these errors were encountered: