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

Support remote-building to macOS hosts #714

Merged
merged 12 commits into from
Nov 10, 2023
Merged

Support remote-building to macOS hosts #714

merged 12 commits into from
Nov 10, 2023

Conversation

grahamc
Copy link
Member

@grahamc grahamc commented Nov 10, 2023

Our README has long featured a snippet to add to the zshenv, with a caveat that it might behave strangely if you're writing a script with an empty PATH.

It is pretty straightforward to eliminate those caveats while still providing remote building for Nix to macOS hosts.

Description
Checklist
  • Formatted with cargo fmt
  • Built with nix build
  • Ran flake checks with nix flake check
  • Added or updated relevant tests (leave unchecked if not applicable)
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • Linked to related issues (leave unchecked if not applicable)
Validating with install.determinate.systems

If a maintainer has added the upload to s3 label to this PR, it will become available for installation via install.determinate.systems:

curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix/pr/$PR_NUMBER | sh -s -- install

Our README has long featured a snippet to add to the zshenv, with a
caevat that it might behave strangely if you're writing a script with an
empty PATH.

It is pretty straightforward to eliminate those caveats while still
providing remote building for Nix to macOS hosts.
@grahamc grahamc added the upload to s3 The labeled PR is allowed to upload its artifacts to S3 for easy testing label Nov 10, 2023
README.md Outdated
@@ -267,6 +267,8 @@ While `nix-installer` tries to provide a comprehensive and unquirky experience,

### Using MacOS remote SSH builders, Nix binaries are not on `$PATH`

*For Nix installations before Determinate Nix Installer version 0.14.1.*
Copy link
Contributor

Choose a reason for hiding this comment

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

This part can be removed, we do not need to document old niche problems in the READMEs of new ones. I suspect we could look into cutting a new release quite soon, even.

Copy link
Member Author

Choose a reason for hiding this comment

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

We might should keep a list of problems and quirks of old releases. Like the below note about nix-darwin should be solved now, now?

Copy link
Contributor

Choose a reason for hiding this comment

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

The old releases have their own READMEs with quirk lists! :)

In the nix-darwin case, users may still try to uninstall Nix on their own before trying to use this tool, and we might get it wrong. We should be able to remove it if we implement a correct fix though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool!

Copy link
Contributor

@lheckemann lheckemann left a comment

Choose a reason for hiding this comment

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

Thanks for jumping on this so quickly! It's a neat approach :)

One corner case I've encountered in the past with the DetSys installer, which this also doesn't cover, is if the user sets their login shell to bash. Not sure if that's common enough to worry about, but I figured it's worth mentioning either way.

@grahamc
Copy link
Member Author

grahamc commented Nov 10, 2023

@lheckemann Hrm. Good to know ... let's see ...

@grahamc
Copy link
Member Author

grahamc commented Nov 10, 2023

@lheckemann Do you have any suggestions as to how to fix that? /etc/profile isn't loaded for that, and I'm not sure what else to try based on a skim of the bash manpage.

@grahamc
Copy link
Member Author

grahamc commented Nov 10, 2023

It works! 🎉

README.md Outdated Show resolved Hide resolved
@grahamc grahamc enabled auto-merge (squash) November 10, 2023 18:42
src/cli/subcommand/repair.rs Outdated Show resolved Hide resolved
@grahamc grahamc merged commit dac0adc into main Nov 10, 2023
13 checks passed
@grahamc grahamc deleted the better-ssh-config branch November 10, 2023 19:02
@lheckemann
Copy link
Contributor

Good point, it looks like the only thing it'll source when it's neither a login shell nor interactive is the file pointed to by $BASH_ENV, which is awkward... I wonder how rude people would find it to mess with the sshd config to use SetEnv.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upload to s3 The labeled PR is allowed to upload its artifacts to S3 for easy testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants