-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
install-nix: Full progress of POSIX installation #1565
Conversation
7cf4f6c
to
d59fdd5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No in-depth review, I just quickly scanned over the code.
Should be also tested on OS X.
d59fdd5
to
b75ea3a
Compare
This is not OS X code, script only for OS X exists already. And since this is fully POSIX, - I am sure it works on macOS flawlessly. There are bridge at the start of the script to macOS. if [[ "$(uname -s)" = "Darwin" ]]; then macOS script is obviously heavy macOS related, and uses Bash. Couple of days ago I wrote to @grahamc that I work on this and I am ready to work togather. Fitting this code to macOS one without merging to master I saw difficult. Because I saw that collaboration should happen. Since I use Linux systems exclusively and not own macOS. I don't know anything about macOS, except that I can 100% rely on And I going to go through macOS code he wrote and get pieces to incorporate. But it also need thought and some rewriting. I see that if merging codebases of Linux and macOS is a tactic that should be looked on, as macOS has many differences. It is not realistic to want to smush them both together in one script, since we are going to overcomplicate things and create technical debt, and half-... solution. Because, then TrueOS, FreeBSD comes, Windows, some other systems come, and we need to support that huge script, and smush that systems into it creating giant complicated monster, or go backwards and untie things from that script. macOS script has a bridge inside this script.
Since I did not LC_COLLATE=C ; export LC_COLLATE
LANG=C ; export LANG
umask 022 You right, that touches macOS, it needs to be looked in, if it is potent to macOS. As I see, we can move bridge at the top, and then we work what can be shared, and what not. I rewrote to: LC_COLLATE=C
LANG=C
umask 022 For now, so it does not get exported into macOS setup. Now, on OS X in must be nothing to test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff! Just some typos and light refactoring…
aha indeed, TIL
…On Tue, Sep 12, 2017 at 1:02 PM Bjørn Forsman ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In scripts/install-nix-from-closure.sh
<#1565 (comment)>:
> +readonly self="$(dirname "$0")"
+readonly ***@***.***@"
+readonly ***@***.***@"
+readonly appname="$0"
+}
+###############################
+### CLI control constants
+###############################
+{
+# tput is not present on some systems (Alpine Linux), this trick with printf allows to store, not 'codes' - literal symbols
+readonly red=$(printf '\033[1;31m')
+readonly green=$(printf '\033[1;32m')
+readonly yellow=$(printf '\033[1;33m')
+readonly blue=$(printf '\033[1;34m')
+readonly bold=$(printf '\033[1m')
+readonly reset=$(printf '\033[0;m') # Reset to default output
@wmertens <https://github.com/wmertens>: Better use "command -v" instead
of "type -p" if you want to be compatible with POSIX sh.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1565 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AADWljhPSqABfkAHVm_QRyZMhHkEFM_aks5shmS1gaJpZM4PSDHd>
.
|
942dbe8
to
953fbde
Compare
|
Tested in real battle on my main complex Arch Linux setup. Knowing that you need to dodge C++ code blocker #697 #1559, that assumes you made multiuser presetup. Install works flawlessly. I encourage everyone to try it. Note: it is beautifully coloured in reality:
|
Looks like a huge amount of work here. A big thank you to everyone involved in this! |
I've made a Docker pipeline that automatically assembles new versions of this installer into So anyone can always try out the current version on it in this pull request. That is the side-product from my presentation at NixCon. ... we got here a small factorio running :]... |
scripts/install-nix-from-closure.sh
Outdated
fi | ||
# macOS support for 10.10 or higher | ||
if [ "$(uname -s)" = 'Darwin' ]; then | ||
if [[ $(($(sw_vers -productVersion | cut -d '.' -f 2))) -lt 10 ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[[ ]]
is a bashism
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already part of Graham MacOS code.
MacOS by default always uses Bash. And Graham's script fully uses Bash.
I not changed this deliberately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better to move to POSIX for this check too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just can't check what:
if [[ $(($(sw_vers -productVersion | cut -d '.' -f 2))) -lt 10 ]]
Parses and does.
I do not understand this Bash:
if [[ $(($(val))) -lt 10 ]]; then
I don't get what is the sense of:
$(($(val)))
I don't know what is $(())
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$(())
is for arithmetic. Eg:
$ x=$((4 + 5));
$ echo "$x"
9
In that case I think that it can be ignored since $(sw_vers -productVersion | cut -d '.' -f 2)
would return a single number. The only difference would be if it didn't return a number. Eg:
$ x=$(($(echo foo)))
$ echo $x
0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is why using logic I thrown my hands.
$(())
must calculate and return number, but I don't know what policy and history Apple has with sw_vers -productVersion
on macOS, and why and how it can happen that it calculates some numbers inside it.
Since I just was laid off at work. I have time, and if someone wants something. Since I got more Nix experience in this time, I can make second shift on this and maybe change something. Provide more clear explanation to people. But I am interested to do Because, nevertheless I know much more of Nix now, I not going to forget the stepping stones they must be simple. If person wants single-user I would be interested if we merge this. As it is now. Or almost as it is. And then I can work on additions. Receive some input, and do some further collaboration. Make options, for active and unattended install. And find a way to source current shell environment. It was been ideal to do everything needed and merge this pull request during Ukrainian holidays. |
So. What should we do with this? It was tested and specifically presented to show that it works long time ago. |
What we do about this. I made this. It is POSIX compatible. Documentation. Comments everywhere. It is reviewed a number of times by number of people. Made testing. Live testing. Live talk. Live demonstration. Explanation. That was done purely to make people sure to merge this. I can update it to master once more. Or I can stop caring. it goes out of mainline fast on every change to upstream - this is shell scripting. So far this can be synced to master. What should I do, update or close this. |
Maybe now that we have more a nix core maintainer team, this increases the pool of people who can review this. |
rm old channel update
rm old message 'performing ... installation', which was to STDERR btw
rm old modify profile
rm old profile messages
1ea3716
to
721cb5f
Compare
I went through all comments on the code, the issues mainly of output text or matter of And almost nobody seems to complain/fix Except for a number of newcomers in Nix issues. I already said previously - I wanted to dedicate myself to support the installers particularly. I am frustrated at the process of merging of this PR. Answering the reviews now become emotional to me. I tried to be as polite as I could. Thank you for understanding. If this ever merged - I would start covering those suggestions. |
Thank you for the reviews, it mattered to me that some people really took the call and helped me. |
And you know that this PR is multiuser install, if I add: groupadd -r nixbld
for n in $(seq 1 10); do useradd -c "Nix build user $n" \
-d /var/empty -g nixbld -G nixbld -M -N -r -s "$(which nologin)" \
nixbld$n; done , right? And you know in most containers where people try to install Nix - there is no And by the way - this snipped is pasted from official docs. |
@Anton-Latukha Your emphasis on the importance of a portable installer is very valuable, as is this PR. Currently I have severe - and in my opinion unnecessary - pains installing Nix on Ubuntu on a VPS, and your PR answers all of them and then some. |
I marked this as stale due to inactivity. → More info |
Still important to me. Not sure why this has stalled. Is there a concern about regression or behavior differences? |
This is also still important for me. But this PR is huge, and if I were responsible I would loathe to review such a big change. And now there even are merge conflicts. Maybe we can come up with a strategy to get this going again. |
Are there any instructions for running this script, please? It seems to be part of the installation tarball, so I'd need to get the last working commit without merge conflicts, or open the tarball and replace install-nix-from-closure.sh, right? Sorry, I didn't look much further after it expected a To the code reviewers: it doesn't seem too complicated reviewing the script as a whole instead of trying to review all the smallest changes... the script is big mostly due to a lot of error checking. |
It is really simple really. The sizes of PRs in repos are proportionate to the ease to merge the improvements (which is proportional to responce times & flexibility of maintainers to hold dialog with contributors & desire to merge improvements). I had 3 PRs with tiny important fixes, they never got merged, so work ended up gathering here. Review of this PR is not difficult at all. Review is not difficult, it requires knowledge of Again, example of the other kind of a topic, that topic I would consider complex & change is just as huge, but the most of the change is inside 1 commit - "try to check that one". When the contributor is not responding to maintainer. In other words, the situation is really not about the code, it is of personel, if there was dialog - the work would continued. There were some changes 1 year ago, where some fixes of things I was fixing here arrived. It was 3-2.5 years of a delay to react to installation issues, but better late then never, agree? In/from that time I know there was a couple of changes in installers & to that time I lost interest, understandably. @teresaejunior Sorry to inform you. Since upstream not collaborated with me on this - there is no way for me to provide support. When this work started & ongoing it was ~4->3 years ago. Since nothing of it was merged, I am not sure the script still aligns with how currently the upstream bootstrapping works. How much time that was? Enough of time to not know Haskell and then know pretty advanced Haskell solidly & do some stuff with with it. That people keep showing up into this thread is sad. I directly to the main upstream people several times proposed to take maintenence of inslallation&deinstallation automatization, atomicity & portability on myself. Simon Payton Jones in the recent talk used particularly a project installation/portability work as an example of a work that is hard to find volunteers for, while the work being of most importance & demanded to be done & maintained at all times. The same was & is obvious to me, so that is why I was astonished to the lack of upstream interest into reports & my propositions & not got any responce nor help. From what I observed I'd say they seem to pretended that all that discussion never existed. So if someone would be able to be compatible with the Nix upstream & would find a foot hold to fix this situation & would want to further Nix installation agenda - I would thank that person, not because I need it, but just for doing it for others, for caring for people. I can not maintain the Nix installer outside of the Nix repository. The effect of it would be - to provide the path to allow to keep the upstream issues unsolved. I suggest to approach those who write the upstream script. I talked to them, but they decided to hold that responcibility tight to themselves, so they bear that responcibility, responcibility is not just badges, fame & generating code. Responcibility is maintenence. They took responcibility - they are respoincible to maintain it at all times. |
If upstream wants - PR can be closed. Otherwise I would close it myself soon, since the tail of a discussion of the PR started to repeat itself. |
|
||
# | ||
# Nix installation script | ||
# Shell script for POSIX-comapible environments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: comapible
.
Closing as it's too big, outdated and the author said he's not interested to continue the work. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/nix-installer-workgroup/21495/13 |
New TODO round:
Done.
Launching merge stage.
Introduction
This is the same script from the current master.
It works the same, and process is the same, that was not touched anyhow.
I've worked on script for a while. Added structure, checks, error handling, features, output functions, so on...
And it transformed to this state.
I tested it. In testing environments. And also used to make Nix setup on my main complex Arch Linux setup.
I am sure in it.
You can test it.
I will be happy to continue work.
Video demonstration:
Current master: https://www.youtube.com/watch?v=NuyevCXBqyY
This commit: https://www.youtube.com/watch?v=yD3pgkMvuGw
Video addresses generic procedure.
Commit also advances installation in cases not shown in video.
Solves:
Solves 'installing as root does not fail (or: prints an over-severe warning) installing as root does not fail (or: prints an over-severe warning) #1125, closes Update wording on root install #1143
Also this was mentioned in install.sh homepage instructions only work without build-user-group #697
sudo
, after downloading the 69Mb archive installation fails while askingsudo
, _after_ downloading the 69Mb archive #539''how to correctly install Nix on ArchLinux how to correctly install Nix on ArchLinux #879'
Embodies
Features
/nix
sudo
installedsudo
access.su
and root access.terminfo
andtput
if available.nix-env --install
andnix-channel --add
) have revert operations. On error - script reverts those actions.trap
.self
resolves to absolute path.Still:
nixbld
group and workers problem from install.sh homepage instructions only work without build-user-group #697, addressed in bugreport Nix single user install on Linux wants 'nixbld' group and workers, else - it fails #1559 'Nix single user install on Linux wants 'nixbld' group and workers, else - it fails'It needs or to be solved, or embodied in official documentation and single-user installation (and also in this script).
Why
printf
used instead ofecho
Shortly, - it is more portable across systems.
https://askubuntu.com/questions/467747/which-is-better-printf-or-echo,
https://unix.stackexchange.com/questions/65803/why-is-printf-better-than-echo,
https://www.in-ulm.de/~mascheck/various/echo+printf/
Why
trap
instead ofset -e
You could came across thought that
set -e
is considered bad practice couple of times.'
set -e
causes the shell to exit if any sub-command or pipeline returns a non-zero status.'. Also: https://stackoverflow.com/questions/19622198/what-does-set-e-mean-in-a-bash-script.So with
set -e
on errors - script freaks-out and exits. And what partially was done - stays partially done on the system. So it creates stateful situation and script can not be transactional.If Nix states to be transactional, so better to maximally work toward transactional install. I think Nix script must handle errors and revert processes, if possible, that is what
trap
s are for.Other
Checked with static analysis.
Tested on Arch Linux, Ubuntu and Alpine Linux in different cases:
There are a lot of considerations and moments to make this script both mature, portable and understandable went to it.
If human knows
shell
, - he can extend this script, or troubleshoot his case from it.It is readable and moments that probably needed explanation - documented.
I plan to mutually exchange code parts with macOS version by @grahamc.
But it needs mention, that macOS and Linux is very different platforms.
macOS is derivative of BSD.
Linux is derivative of Minix and Unix.
macOS and Linux is completely different internally, as far as possible, systems, both have more in common with Unix, then some mutual similarity internally. Please don't ask to deduplicate scripts completely, or merge them, that going to be overcomplication of process and not readable/maintainable. That requirement also going to degrade macOS part, since lowest denominator of POSIX must be used then. While macOS platform is much more homogeneous, let that be the strength of the platform.
macOS and Linux have many differences regarding commands and environment, that is why processes here should be or separate or flexible. Flexible is what I aim for. To exchange what is possible, and bring scripts to the huge level.
I am ready to maintain Linux installation of Nix. If not, - I ready to contribute this and move further.
Backlog
terminfo
,tput colors
,termcap
,tput Co
,$TERM
and$SHELL
special cases.Testing platform
Just for this PR the testing platform was created: https://github.com/Anton-Latukha/nix-tests-terraform
, which covers the most different 9 Linux distributions:
groupadd
oruseradd
- Alpine based on BusyBox.And it was tested that on all these distributions this installer works.
I even spent a work week on making automated BSD testing, creating them KVM machines, resolving them BSD compatibility issues, learning different BSD systems, and testing this on all major of them.
In BSD installation fails because of upstream hardcode issue: #1559, and I still didn't got any information back why is it so. If not for Linux-specific hardcoded groups - the installer would install on BSDs. The official support is the other thing, we do not claim it here. But maybe they would be interested themselves in some Nix and would start supporting/helping with it themselves.
This change is