NixOS / experimental-nix-installer

An experimental fork of the Determinate Nix Installer to explore upstreaming.
GNU Lesser General Public License v2.1
31 stars 2 forks source link

Merge upstream #15

Closed mkenigs closed 5 months ago

mkenigs commented 5 months ago
mkenigs commented 5 months ago

The merge was definitely non-trivial so I'd recommend reviewing how GitHub displays the merge commit https://github.com/NixOS/experimental-nix-installer/pull/15/commits/4993d8c94679087033ed9df327e44ac4773df38b or using something like git show --diff-merges=r

abathur commented 5 months ago

Apologies for sluggishness here. I was planning to get to this Friday morning, but Thursday evening mother nature threw us a power-outage that has yet to resolve. Hopefully Tuesday or Wednesday, by current projections.

With the big caveat that I'm fairly distracted:

mkenigs commented 5 months ago

Apologies for sluggishness here. I was planning to get to this Friday morning, but Thursday evening mother nature threw us a power-outage that has yet to resolve. Hopefully Tuesday or Wednesday, by current projections.

With the big caveat that I'm fairly distracted:

* I looked at the merge diff as you suggest and I think all of that makes sense.
  The only real note I had is that the overall diff might be a little smaller if we just disabled the enterprise flag at the CLI and hard-set the value for it to false instead of rewriting the conditionals? (It probably doesn't make sense to waste a cycle here on it since we can just refactor it later. If you agree I'll probably just open an issue for it with some links--but maybe you'll disagree about preserving the conditionals?)

So that would basically mean compiling support for enterprise_edition but always disabling it? My thought process was to completely avoid compiling that code. But the DetSys default value for enterprise_edition is false, so we could even just leave it in completely. Probably worth discussing sync.

abathur commented 5 months ago

Per sync: avoiding compilation sounds like a good reason to me. Can always revisit the diff-minimization part later if it gets annoying.