wslconfig: write autoMemoryReclaim under [experimental], where WSL reads it

CHANGELOG.md

@@ -12,6 +12,17 @@ useful than saying where the real one is.
 
 ## [Unreleased]
 
+### Fixed
+
+- **`wsl.auto-memory-reclaim` now takes effect** ([#550](https://github.com/wslkit/skrog/issues/550)).
+  `skrog wsl-config apply` wrote `autoMemoryReclaim` under `[wsl2]`, but WSL reads it only
+  as `experimental.autoMemoryReclaim`: it ignored the line and printed `Unknown key
+  'wsl2.autoMemoryReclaim'` when the VM started, so the setting never worked. It is now
+  written under `[experimental]`, and a line an earlier release left under `[wsl2]` is
+  moved by the next `apply` — the diff shows the move before anything is written. `skrog
+  wsl-config` and `skrog top` no longer report the ignored line as in effect.
+  `wsl-config --json` pending changes gain `section`, and `moved_from` for a move.
+
 ### Added
 
 - **An engine per project** ([#539](https://github.com/wslkit/skrog/issues/539)).

cmd/skrog/jsonshapes.go

@@ -342,6 +342,10 @@ type wslConfigChangeJSON struct {
 	Old   string `json:"old,omitempty"`
 	New   string `json:"new"`
 	Added bool   `json:"added"`
+	// Section is where the key is written: wsl2, or experimental (#550).
+	Section string `json:"section"`
+	// MovedFrom names the section a dead copy is removed from; absent otherwise.
+	MovedFrom string `json:"moved_from,omitempty"`
 }
 
 // statsJSON is the `stats` object `skrog status --stats --json` adds (#179).

cmd/skrog/jsonshapes_test.go

@@ -269,7 +269,7 @@ func TestWSLConfigShape(t *testing.T) {
 		t.Error("pending should be omitted when there is nothing to do")
 	}
 	c := roundTrip(t, wslConfigChangeJSON{Key: "memory", New: "4GB", Added: true})
-	requireKeys(t, c, "key", "new", "added")
+	requireKeys(t, c, "key", "new", "added", "section")
 	if _, ok := c["old"]; ok {
 		t.Error("old should be omitted when the key is being added")
 	}

cmd/skrog/top.go

@@ -309,8 +309,9 @@ func renderTop(w io.Writer, s vmtop.Snapshot, reclaim string) {
 	}
 	if advised {
 		if reclaim == "" {
-			fmt.Fprintln(w, "~/.wslconfig does not set autoMemoryReclaim; `skrog config set wsl.auto-memory-reclaim gradual`\n"+
-				"asks WSL to hand idle memory back. See docs/vm-sizing.md.")
+			fmt.Fprintln(w, "~/.wslconfig does not set [experimental] autoMemoryReclaim; `skrog config set\n"+
+				"wsl.auto-memory-reclaim gradual` then `skrog wsl-config apply` asks WSL to hand idle\n"+
+				"memory back. See docs/vm-sizing.md.")
 		} else {
 			fmt.Fprintf(w, "~/.wslconfig sets autoMemoryReclaim=%s. See docs/vm-sizing.md.\n", reclaim)
 		}

cmd/skrog/wslconfigcmd.go

@@ -105,6 +105,7 @@ func runWSLConfigShow(args []string) int {
 		for _, c := range plan {
 			out.Pending = append(out.Pending, wslConfigChangeJSON{
 				Key: c.Key, Old: c.Old, New: c.New, Added: c.Added,
+				Section: wslconfig.SectionOf(c.Key), MovedFrom: c.StaleSection,
 			})
 		}
 		return emitJSON(out)
@@ -198,6 +199,7 @@ func runWSLConfigApply(args []string) int {
 		for _, c := range plan {
 			out.Pending = append(out.Pending, wslConfigChangeJSON{
 				Key: c.Key, Old: c.Old, New: c.New, Added: c.Added,
+				Section: wslconfig.SectionOf(c.Key), MovedFrom: c.StaleSection,
 			})
 		}
 		return emitJSON(out)

docs/cli-json.md

@@ -600,11 +600,17 @@ The WSL2 VM''s sizing, from the global `~/.wslconfig` (#148):
   "exists": true,
   "effective": { "memory": "4GB", "processors": "2", "autoMemoryReclaim": "gradual" },
   "desired":   { "memory": "4GB", "processors": "2", "autoMemoryReclaim": "gradual" },
-  "pending": [ { "key": "memory", "old": "8GB", "new": "4GB", "added": false } ],
+  "pending": [ { "key": "memory", "old": "8GB", "new": "4GB", "added": false, "section": "wsl2" } ],
   "applied": false
 }
 ```
 
+- `effective` reads each key from the section WSL reads it from:
+  `autoMemoryReclaim` from `[experimental]`, the rest from `[wsl2]` (#550).
+- Each `pending` change names its `section`. `moved_from` is present when a
+  copy WSL ignores is being removed from another section — an
+  `autoMemoryReclaim` an earlier release wrote under `[wsl2]`.
+
 - `effective` is what the file says now; `desired` is what Skrog's own
   settings ask for; `pending` is the difference — so a converge script can tell
   "already right" from "would change something" without parsing prose.

docs/memory.md

@@ -171,8 +171,9 @@ held on the host:
 ```
 Windows holds 288.3 MiB more for the VM than the VM is using: memory freed inside
 the VM that has not been handed back yet.
-~/.wslconfig does not set autoMemoryReclaim; `skrog config set wsl.auto-memory-reclaim gradual`
-asks WSL to hand idle memory back. See docs/vm-sizing.md.
+~/.wslconfig does not set [experimental] autoMemoryReclaim; `skrog config set
+wsl.auto-memory-reclaim gradual` then `skrog wsl-config apply` asks WSL to hand idle
+memory back. See docs/vm-sizing.md.
 ```
 
 The two figures are different measures (Windows' private working set, the
@@ -190,8 +191,9 @@ holds — `top` says so:
 ```
 3.0 GiB of the VM's memory is page cache: file data Linux will drop when it needs
 the room, but which Windows counts as Vmmem until it is reclaimed.
-~/.wslconfig does not set autoMemoryReclaim; `skrog config set wsl.auto-memory-reclaim gradual`
-asks WSL to hand idle memory back. See docs/vm-sizing.md.
+~/.wslconfig does not set [experimental] autoMemoryReclaim; `skrog config set
+wsl.auto-memory-reclaim gradual` then `skrog wsl-config apply` asks WSL to hand idle
+memory back. See docs/vm-sizing.md.
 ```
 
 That cache is not a leak and nothing needs killing: Linux gives it up the

docs/vm-sizing.md

@@ -21,9 +21,21 @@ skrog wsl-config apply          # shows the diff, asks, then writes
 | `wsl.memory` | `memory` | cap on the VM's RAM (`4GB`, `512MB`) |
 | `wsl.processors` | `processors` | how many CPUs the VM sees |
 | `wsl.swap` | `swap` | swap file size; `0` disables it |
-| `wsl.auto-memory-reclaim` | `autoMemoryReclaim` | `gradual`, `dropcache` or `disabled` — hands idle memory back to Windows |
+| `wsl.auto-memory-reclaim` | `autoMemoryReclaim`, under **`[experimental]`** | `gradual`, `dropcache` or `disabled` — hands idle memory back to Windows |
 | `wsl.virtiofs` | `virtiofs` | `true` mounts Windows drives over virtiofs instead of 9p — see below |
 
+Every key goes under `[wsl2]` except `autoMemoryReclaim`, which WSL reads only
+as `experimental.autoMemoryReclaim`. Under `[wsl2]` it is ignored, and WSL
+prints `Unknown key 'wsl2.autoMemoryReclaim'` the next time the VM starts —
+which is where Skrog wrote it before
+[#550](https://github.com/wslkit/skrog/issues/550). If an earlier release left
+it there, `skrog wsl-config` shows the move and `apply` makes it:
+
+```
+  - autoMemoryReclaim=gradual  (under [wsl2], where WSL ignores it)
+  + autoMemoryReclaim=gradual  (under [experimental])
+```
+
 Values are validated when you set them, in the way WSL reads them, so a typo
 fails at `skrog config set` rather than silently sizing the VM as something
 else. `memory=4` in `.wslconfig` means *four bytes*; `skrog config set
@@ -108,7 +120,7 @@ C:\Users\me\.wslconfig is shared by every WSL2 distro on this machine.
 Skrog would change:
   + memory=4GB
   + processors=2
-  + autoMemoryReclaim=gradual
+  + autoMemoryReclaim=gradual  (under [experimental])
 
 apply these changes? [y/N]
 ```

internal/wslconfig/wslconfig.go

@@ -29,9 +29,35 @@ import (
 	"strings"
 )
 
-// Section is the only section this package writes into.
+// Section is where a managed key lives unless SectionOf says otherwise.
 const Section = "wsl2"
 
+// SectionExperimental holds the keys WSL has not promoted to [wsl2].
+const SectionExperimental = "experimental"
+
+// SectionOf is the section WSL reads a managed key from.
+//
+// autoMemoryReclaim is `experimental.autoMemoryReclaim` in WSL's own config
+// table, with no `wsl2.` alias (networkingMode has both). Written under [wsl2]
+// it is an "Unknown key" WSL ignores, which is what every Skrog release before
+// #550 did.
+func SectionOf(key string) string {
+	if strings.EqualFold(key, KeyAutoMemoryReclaim) {
+		return SectionExperimental
+	}
+	return Section
+}
+
+// staleSection is where an earlier Skrog wrote a key WSL does not read there,
+// or "" when the key was always written to the right place. Plan moves such a
+// line rather than leaving a dead one behind.
+func staleSection(key string) string {
+	if strings.EqualFold(key, KeyAutoMemoryReclaim) {
+		return Section
+	}
+	return ""
+}
+
 // Keys are the settings Skrog manages, in the order a diff shows them.
 const (
 	KeyMemory            = "memory"
@@ -99,7 +125,9 @@ func Load(path string) (*File, error) {
 // Existed reports whether the file was there before.
 func (f *File) Existed() bool { return f.existed }
 
-// Get returns a key's current value in [wsl2] and whether it was set.
+// Get returns a key's current value in the section WSL reads it from, and
+// whether it was set there. A copy in any other section is not reported: WSL
+// ignores it, so it is not the effective value.
 func (f *File) Get(key string) (string, bool) {
 	i := f.find(key)
 	if i < 0 {
@@ -121,14 +149,19 @@ func (f *File) All() map[string]string {
 	return out
 }
 
-// find locates a key inside the [wsl2] section, or -1. Comparison is
-// case-insensitive on both the section and the key, because WSL is.
+// find locates a key inside the section WSL reads it from, or -1.
 func (f *File) find(key string) int {
+	return f.findIn(SectionOf(key), key)
+}
+
+// findIn locates a key inside the named section, or -1. Comparison is
+// case-insensitive on both the section and the key, because WSL is.
+func (f *File) findIn(section, key string) int {
 	inSection := false
 	for i, line := range f.lines {
 		t := strings.TrimSpace(line)
 		if strings.HasPrefix(t, "[") && strings.HasSuffix(t, "]") {
-			inSection = strings.EqualFold(strings.Trim(t, "[]"), Section)
+			inSection = strings.EqualFold(strings.Trim(t, "[]"), section)
 			continue
 		}
 		if !inSection || t == "" || strings.HasPrefix(t, "#") || strings.HasPrefix(t, ";") {
@@ -157,14 +190,35 @@ type Change struct {
 	New string
 	// Added is true when the key is not in the file yet.
 	Added bool
+	// StaleSection names the section a dead copy of the key is removed from
+	// (an earlier Skrog wrote it where WSL does not read it); empty otherwise.
+	StaleSection string
+	// Stale is the value on that dead line.
+	Stale string
 }
 
 // String renders a change as a diff line, which is what consent is given to.
+// A key outside [wsl2] names its section, and a moved line says why, because
+// "- x / + x" with the same value would otherwise read as a no-op.
 func (c Change) String() string {
+	where := ""
+	if s := SectionOf(c.Key); s != Section {
+		where = "  (under [" + s + "])"
+	}
+	var b strings.Builder
+	if c.StaleSection != "" {
+		fmt.Fprintf(&b, "- %s=%s  (under [%s], where WSL ignores it)\n", c.Key, c.Stale, c.StaleSection)
+	}
 	if c.Added {
-		return fmt.Sprintf("+ %s=%s", c.Key, c.New)
+		fmt.Fprintf(&b, "+ %s=%s%s", c.Key, c.New, where)
+		return b.String()
 	}
-	return fmt.Sprintf("- %s=%s\n+ %s=%s", c.Key, c.Old, c.Key, c.New)
+	if strings.EqualFold(c.Old, c.New) {
+		// Only the dead copy goes; the live line is already right.
+		return strings.TrimSuffix(b.String(), "\n")
+	}
+	fmt.Fprintf(&b, "- %s=%s%s\n+ %s=%s%s", c.Key, c.Old, where, c.Key, c.New, where)
+	return b.String()
 }
 
 // Plan returns the changes needed to make the managed keys in the file match
@@ -178,12 +232,17 @@ func (f *File) Plan(desired map[string]string) []Change {
 			continue // unset in Skrog's config: not ours to touch
 		}
 		cur, present := f.Get(k)
-		switch {
-		case !present:
-			out = append(out, Change{Key: k, New: want, Added: true})
-		case !strings.EqualFold(cur, want):
-			out = append(out, Change{Key: k, Old: cur, New: want})
+		c := Change{Key: k, Old: cur, New: want, Added: !present}
+		if s := staleSection(k); s != "" {
+			if i := f.findIn(s, k); i >= 0 {
+				c.StaleSection = s
+				_, c.Stale = splitKV(f.lines[i])
+			}
+		}
+		if present && strings.EqualFold(cur, want) && c.StaleSection == "" {
+			continue
 		}
+		out = append(out, c)
 	}
 	return out
 }
@@ -196,12 +255,17 @@ func (f *File) Apply(changes []Change) error {
 		return nil
 	}
 	for _, c := range changes {
+		if c.StaleSection != "" {
+			if i := f.findIn(c.StaleSection, c.Key); i >= 0 {
+				f.lines = append(f.lines[:i], f.lines[i+1:]...)
+			}
+		}
 		if i := f.find(c.Key); i >= 0 {
 			// Keep any inline comment: someone wrote it for a reason.
 			f.lines[i] = c.Key + "=" + c.New + inlineComment(f.lines[i])
 			continue
 		}
-		f.insertIntoSection(c.Key + "=" + c.New)
+		f.insertIntoSection(SectionOf(c.Key), c.Key+"="+c.New)
 	}
 	return f.write()
 }
@@ -214,18 +278,18 @@ func inlineComment(line string) string {
 	return ""
 }
 
-// insertIntoSection appends a line at the end of [wsl2], creating the section
-// when the file has none. Appending rather than sorting keeps the user's own
-// ordering intact.
-func (f *File) insertIntoSection(line string) {
+// insertIntoSection appends a line at the end of the named section, creating
+// the section when the file has none. Appending rather than sorting keeps the
+// user's own ordering intact.
+func (f *File) insertIntoSection(section, line string) {
 	start := -1
 	end := len(f.lines)
 	for i, l := range f.lines {
 		t := strings.TrimSpace(l)
 		if !strings.HasPrefix(t, "[") || !strings.HasSuffix(t, "]") {
 			continue
 		}
-		if strings.EqualFold(strings.Trim(t, "[]"), Section) {
+		if strings.EqualFold(strings.Trim(t, "[]"), section) {
 			start = i
 			end = len(f.lines)
 			continue
@@ -236,11 +300,11 @@ func (f *File) insertIntoSection(line string) {
 		}
 	}
 	if start < 0 {
-		// No [wsl2] at all: add one, separated from whatever precedes it.
+		// No such section at all: add one, separated from whatever precedes it.
 		if len(f.lines) > 0 && strings.TrimSpace(f.lines[len(f.lines)-1]) != "" {
 			f.lines = append(f.lines, "")
 		}
-		f.lines = append(f.lines, "["+Section+"]", line)
+		f.lines = append(f.lines, "["+section+"]", line)
 		return
 	}
 	// Insert after the section's last non-blank line, so a trailing blank line