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

programs.nh: init module #942

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

Conversation

alanpearce
Copy link

Based on the NixOS module of the same name.

I used the same checks that nix.gc uses, as it performs exactly that function.

@bestlem bestlem mentioned this pull request May 1, 2024
@skykanin
Copy link

Does the nh CLI even support nix-darwin?

@alanpearce
Copy link
Author

Does the nh CLI even support nix-darwin?

Yes? I wouldn't have worked on this otherwise. Only the os subcommand doesn't work

@viperML
Copy link

viperML commented May 22, 2024

nh doesn't wrap nix-darwin. And I don't use macOs, so thought has not been put into making sure the rest works there.

@alanpearce
Copy link
Author

nh doesn't wrap nix-darwin. And I don't use macOs, so thought has not been put into making sure the rest works there.

Could you explain why you said that? I'm not sure what point you're trying to make.

@viperML
Copy link

viperML commented May 22, 2024

I don't know what is to explain, nh doesn't support nix-darwin in the sense that it doesn't have anything to do with nix-darwin, not nix on darwin.

@ToyVo
Copy link

ToyVo commented May 22, 2024

The nh os switch does not support Darwin due to differences between nixos-rebuild and darwin-rebuild. I have a fork that adds support for darwin. https://github.com/ToyVo/nh

The nh clean command does work on Darwin on the original and my fork.

@alanpearce
Copy link
Author

I don't know what is to explain, nh doesn't support nix-darwin in the sense that it doesn't have anything to do with nix-darwin, not nix on darwin.

I think perhaps the meaning of "support" here is overloaded. I'll try to clarify, please correct me if I'm wrong:

  • nh works on darwin as in I can install the package and run it and it works, except:
  • the os subcommand does not work on darwin as you don't support darwin and don't want to, which is totally fine.

As nh has more functionality than the os subcommand, it is reasonable to include in nix-darwin. All the functionality of nh exposed as options for nix-darwin in this PR work—that is, there is no functionality here that relies on the os subcommand.

@bestlem
Copy link

bestlem commented May 23, 2024

@alanpearce

My understanding is that nh was written by @viperML and this version does not support darwin as @viperML does not have a Mac to be able to test.
@ToyVo has produced a fork of the original nh and this does support darwin ie os switch works. (I have used this and it does work).

The nh module here can be made to use @ToyVo s fork and so is fully functional on macOS. By setting programs.nh.package

@viperML
Copy link

viperML commented May 23, 2024

To add the comment from @bestlem, there are 2 side of the "not supported":

A) Not supporting darwin as the run-time platform for nh home or nh clean. I guess they work fine but I also don't know any of the corner cases for darwin.
B) Not having a nix-darwin wrapper, which is done in the fork.

@alanpearce
Copy link
Author

@viperML I'm still not sure what the point is you're trying to make. What is the reason for you saying that darwin isn't supported? What do you hope to achieve? Is there something that you think I should do with regard to this PR?

@bestlem
Copy link

bestlem commented May 23, 2024

@alanpearce What @viperML is saying that his code might or might not run on darwin and if you find an issue with his original on darwin he can't investigate so can't fix it.

You run his executable at your own risk and the author can't answer anything about issues.

@viperML is not being awkward here they are just saying that they can't deal with darwin issues. This is fair as they can't test,

To test things you have to have all the different hardware and operating systems that your customers use. You can do this if you are reasonably sized organisation but no-one can expect single person FOSS developers to do this. This is no doubt one of the reason nix-darwin exists it can deal with issues that are different to nixes.

@SolidRhino
Copy link

@LnL7 when will you merge this? This is a great edition.

@ToyVo
Copy link

ToyVo commented Jun 4, 2024

@SolidRhino the nh-darwin fork does have a Darwin module that defines the same options if you are impatient for this pr.

I have also recently closed an issue that pointed out that there was indeed a bug with nh os clean on macos that I have since fixed. That bug was macos uids start at 501 so the original nh would clean xdg directories for users 1000-1100, now on nh-dawrin the appropriate users 501-601 are cleaned.

@kamushadenes
Copy link
Contributor

Hey, can we get this merged? Thanks!

@zhongjis
Copy link

zhongjis commented Oct 8, 2024

Wondering what is blocking us from merging this PR? If possible can any of the maintainers providing some specifications or concerns so we can address them?

@kimjongbing
Copy link

kimjongbing commented Oct 12, 2024

Wondering what is blocking us from merging this PR? If possible can any of the maintainers providing some specifications or concerns so we can address them?

someone needs to get the maintainer a macbook lol or a vm

@bestlem
Copy link

bestlem commented Oct 13, 2024

@kimjongbing No - this refers to a fork for the macOS version by ToyVo

options.programs.nh = {
enable = mkEnableOption "nh, yet another Nix CLI helper";

package = mkPackageOption pkgs "nh" { };

Choose a reason for hiding this comment

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

Instead could we use https://github.com/ToyVo/nh_darwin instead of the default nh package as it doesn't support darwin-rebuild when nh_darwin does!

Perhaps making it a input?

Copy link
Author

Choose a reason for hiding this comment

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

That would make sense, but requires nh_darwin to be in nixpkgs. As it is, users will be able to choose to use that package if they have imported it.

Copy link

Choose a reason for hiding this comment

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

nh makes errors under darwin - darwin has to use the nh_darwin fork.

For example there are checks on the user id - nh uses the Linux rules and so only checks for ids >= 1000 nh_darwin understands that macOS users start at 501. This affect nh clean as far as I remember

I don't think there is any point in a darwin module that does not use the nh_darwin fork.

Copy link

Choose a reason for hiding this comment

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

Also the nh_darwin fork does provide modules for darwinConfiguration and homeConfiguration whey does nix-darwin need to provide a module?

Copy link

@Eveeifyeve Eveeifyeve Nov 16, 2024

Choose a reason for hiding this comment

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

That would make sense, but requires nh_darwin to be in nixpkgs. As it is, users will be able to choose to use that package if they have imported it.

I agree with this, @ToyVo or someone here Are you willing to make the package nh_darwin in nixpkgs?

Copy link

Choose a reason for hiding this comment

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

I'm hesitant to try to push for nh_darwin to go into nixpkgs as is for the simple fact that its not just an alternative for darwin and linux has my additional features as well. Thinking about it, I think if I were to rebrand to nh_plus or something OS neutral I would comfortable doing that, with a modified versioning scheme of using a 4th segment to indicate each of my patches over base nh for tags

Copy link

Choose a reason for hiding this comment

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

I have migrated to nh_plus, I've made the first tag of 3.6.0-0. This puts it in a better spot to bring into nixpkgs

Choose a reason for hiding this comment

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

I will make the package later today, as I am at school.

Copy link

Choose a reason for hiding this comment

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

I didn't see a pr for it so NixOS/nixpkgs#357106

Copy link

Choose a reason for hiding this comment

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

Change of plans, with base nh 4.0 nix-darwin support has been added. nh 4.0 is less flake-centric and updating flake inputs has been removed so I might keep my fork alive for that if it isn't re-added.

@ToyVo
Copy link

ToyVo commented Nov 20, 2024

Just reiterating outside of the last thread, with nh 4.0 nix-darwin is now supported. So when that update hits nixpkgs this PR will be good to merge

DavSanchez added a commit to DavSanchez/nix-dotfiles that referenced this pull request Dec 8, 2024
DavSanchez added a commit to DavSanchez/nix-dotfiles that referenced this pull request Dec 8, 2024
DavSanchez added a commit to DavSanchez/nix-dotfiles that referenced this pull request Dec 8, 2024
* flake.lock: Update

* feat: nh with darwin support

to remove once LnL7/nix-darwin#942
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.

10 participants