From 609370699f2dc90988f8a6881837c5092484e9c0 Mon Sep 17 00:00:00 2001
From: polykernel <81340136+polykernel@users.noreply.github.com>
Date: Wed, 24 Nov 2021 19:09:53 -0500
Subject: [PATCH] home-manager: do not build news when using flake (#2501)

Currently, the `buildNews` and `doBuildAttrs` are always called
unconditionally even if a flake configuration is specified. This cause
it to always fail prior to the actual build performed by `doBuildAttrs`
because `setConfigFIle` can not find the home-manager configuration file.
As a result, an error message specifying no configuration file is shown.

Furthermore, if a user has remnant legacy configuration, the `doSwitch` and
`doBuild` functions will effectively build the activationPackage twice, with
the legacy configuration overriding the flake configuration.

A conditional check for FLAKE_CONFIG_URI was added to mitigate this by building
the legacy configuration when no flake configuration is present. There is one
exception which is when a flake configuration exists in the default location, where
the user can not build the legacy configuration as along as the file is present.
However, the tradeoff is acceptable as it matches current behavior when FLAKe_CONFIG_URI
is set for instantiation, and an user is unlikely to simulataneously switch
between the two mechanisms.

An abstract function for building flakes `doBuildFlake` was created to match
`doBuildAttrs` for  manageing options and build flags.

The --no-write-lock-file flag was removed from the --debug case as it is already
matched previously at the --recreate-lock-file case.
---
 home-manager/home-manager | 94 ++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 42 deletions(-)

diff --git a/home-manager/home-manager b/home-manager/home-manager
index deabe70c9..990e1e5e8 100644
--- a/home-manager/home-manager
+++ b/home-manager/home-manager
@@ -186,6 +186,18 @@ function doBuildAttr() {
         --argstr confAttr "$HOME_MANAGER_CONFIG_ATTRIBUTE"
 }
 
+function doBuildFlake() {
+    local extraArgs=("$@")
+
+    if [[ -v VERBOSE ]]; then
+        extraArgs=("${extraArgs[@]}" "--verbose")
+    fi
+
+    nix build \
+        "${extraArgs[@]}" \
+        "${PASSTHROUGH_OPTS[@]}"
+}
+
 # Presents news to the user. Takes as argument the path to a "news
 # info" file as generated by `buildNews`.
 function presentNews() {
@@ -246,43 +258,31 @@ function doBuild() {
         return 1
     fi
 
-    setFlakeAttribute
-    if [[ -v FLAKE_CONFIG_URI ]]; then
-        nix build \
-            "${PASSTHROUGH_OPTS[@]}" \
-             ${DRY_RUN+--dry-run} \
-             ${NO_OUT_LINK+--no-link} \
-            "$FLAKE_CONFIG_URI.activationPackage" \
-            || return
-    fi
-
     setWorkDir
 
-    local newsInfo
-    newsInfo=$(buildNews)
-
-    doBuildAttr \
-                ${NO_OUT_LINK+--no-out-link} \
-                --attr activationPackage \
+    setFlakeAttribute
+    if [[ -v FLAKE_CONFIG_URI ]]; then
+        doBuildFlake \
+            "${DRY_RUN+--dry-run} \
+            "${NO_OUT_LINK+--no-link} \
+            "$FLAKE_CONFIG_URI.activationPackage" \
+            || return
+    else
+        doBuildAttr \
+            ${NO_OUT_LINK+--no-out-link} \
+            --attr activationPackage \
         || return
 
-    presentNews "$newsInfo"
+        local newsInfo
+        newsInfo=$(buildNews)
+
+        presentNews "$newsInfo"
+    fi
 }
 
 function doSwitch() {
-    setFlakeAttribute
-    if [[ -v FLAKE_CONFIG_URI ]]; then
-        nix run \
-            "${PASSTHROUGH_OPTS[@]}" \
-            "$FLAKE_CONFIG_URI.activationPackage" \
-            || return
-    fi
-
     setWorkDir
 
-    local newsInfo
-    newsInfo=$(buildNews)
-
     local generation
 
     # Build the generation and run the activate script. Note, we
@@ -291,12 +291,23 @@ function doSwitch() {
     # before activation completes.
     generation="$WORK_DIR/generation"
 
-    doBuildAttr \
-                --out-link "$generation" \
-                --attr activationPackage \
-        && "$generation/activate" || return
+    setFlakeAttribute
+    if [[ -v FLAKE_CONFIG_URI ]]; then
+        doBuildFlake \
+            --out-link "$generation" \
+            "$FLAKE_CONFIG_URI.activationPackage" \
+            && "$generation/activate" || return
+    else
+        doBuildAttr \
+            --out-link "$generation" \
+            --attr activationPackage \
+            && "$generation/activate" || return
 
-    presentNews "$newsInfo"
+        local newsInfo
+        newsInfo=$(buildNews)
+
+        presentNews "$newsInfo"
+    fi
 }
 
 function doListGens() {
@@ -393,13 +404,13 @@ function buildNews() {
     output="$WORK_DIR/news-info.sh"
 
     doBuildAttr \
-                --out-link "$output" \
-                --no-build-output \
-                --quiet \
-                --arg check false \
-                --argstr newsReadIdsFile "$(newsReadIdsFile)" \
-                --attr newsInfo \
-                > /dev/null
+        --out-link "$output" \
+        --no-build-output \
+        --quiet \
+        --arg check false \
+        --argstr newsReadIdsFile "$(newsReadIdsFile)" \
+        --attr newsInfo \
+        > /dev/null
 
     echo "$output"
 }
@@ -616,8 +627,7 @@ while [[ $# -gt 0 ]]; do
             shift
             ;;
         --debug|--keep-failed|--keep-going|--show-trace\
-            |--substitute|--no-substitute|--impure\
-            |--no-write-lock-file)
+            |--substitute|--no-substitute|--impure)
             PASSTHROUGH_OPTS+=("$opt")
             ;;
         -v|--verbose)