feature/featuretags,cmd/omitsize: support feature dependencies

This produces the following omitsizes output:

    Starting with everything and removing a feature...

    tailscaled tailscale combined (linux/amd64)
     27005112  18153656  39727288
    - 7696384 - 7282688 -19607552 .. remove *
    -  167936 -  110592 -  245760 .. remove acme
    - 1925120 -       0 - 7340032 .. remove aws
    -    4096 -       0 -    8192 .. remove bird
    -   20480 -   12288 -   32768 .. remove capture
    -       0 -   57344 -   61440 .. remove completion
    -  249856 -  696320 -  692224 .. remove debugeventbus
    -   12288 -    4096 -   24576 .. remove debugportmapper
    -       0 -       0 -       0 .. remove desktop_sessions
    -  815104 -    8192 -  544768 .. remove drive
    -   65536 -  356352 -  425984 .. remove kube
    -  233472 -  286720 -  311296 .. remove portmapper (and debugportmapper)
    -   90112 -       0 -  110592 .. remove relayserver
    -  655360 -  712704 -  598016 .. remove serve (and webclient)
    -  937984 -       0 -  950272 .. remove ssh
    -  708608 -  401408 -  344064 .. remove syspolicy
    -       0 - 4071424 -11132928 .. remove systray
    -  159744 -   61440 -  225280 .. remove taildrop
    -  618496 -  454656 -  757760 .. remove tailnetlock
    -  122880 -       0 -  131072 .. remove tap
    -  442368 -       0 -  483328 .. remove tpm
    -   16384 -       0 -   20480 .. remove wakeonlan
    -  278528 -  368640 -  286720 .. remove webclient

    Starting at a minimal binary and adding one feature back...

    tailscaled tailscale combined (linux/amd64)
     19308728  10870968  20119736 omitting everything
    +  352256 +  454656 +  643072 .. add acme
    + 2035712 +       0 + 2035712 .. add aws
    +    8192 +       0 +    8192 .. add bird
    +   20480 +   12288 +   36864 .. add capture
    +       0 +   57344 +   61440 .. add completion
    +  262144 +  274432 +  266240 .. add debugeventbus
    +  344064 +  118784 +  360448 .. add debugportmapper (and portmapper)
    +       0 +       0 +       0 .. add desktop_sessions
    +  978944 +    8192 +  991232 .. add drive
    +   61440 +  364544 +  425984 .. add kube
    +  331776 +  110592 +  335872 .. add portmapper
    +  122880 +       0 +  102400 .. add relayserver
    +  598016 +  155648 +  737280 .. add serve
    + 1142784 +       0 + 1142784 .. add ssh
    +  708608 +  860160 +  720896 .. add syspolicy
    +       0 + 4079616 + 6221824 .. add systray
    +  180224 +   65536 +  237568 .. add taildrop
    +  647168 +  393216 +  720896 .. add tailnetlock
    +  122880 +       0 +  126976 .. add tap
    +  446464 +       0 +  454656 .. add tpm
    +   20480 +       0 +   24576 .. add wakeonlan
    + 1011712 + 1011712 + 1138688 .. add webclient (and serve)

Fixes #17139

Change-Id: Ia91be2da00de8481a893243d577d20e988a0920a
Signed-off-by: Brad Fitzpatrick <[email protected]>

cmd/featuretags/featuretags.go

@@ -14,6 +14,7 @@ import (
 	"strings"
 
 	"tailscale.com/feature/featuretags"
+	"tailscale.com/util/set"
 )
 
 var (
@@ -38,7 +39,9 @@ func main() {
 	var keep = map[featuretags.FeatureTag]bool{}
 	for t := range strings.SplitSeq(*add, ",") {
 		if t != "" {
-			keep[featuretags.FeatureTag(t)] = true
+			for ft := range featuretags.Requires(featuretags.FeatureTag(t)) {
+				keep[ft] = true
+			}
 		}
 	}
 	var tags []string
@@ -55,6 +58,7 @@ func main() {
 			}
 		}
 	}
+	removeSet := set.Set[featuretags.FeatureTag]{}
 	for v := range strings.SplitSeq(*remove, ",") {
 		if v == "" {
 			continue
@@ -63,7 +67,16 @@ func main() {
 		if _, ok := features[f]; !ok {
 			log.Fatalf("unknown feature %q in --remove", f)
 		}
-		tags = append(tags, f.OmitTag())
+		removeSet.Add(f)
+	}
+	for ft := range removeSet {
+		set := featuretags.RequiredBy(ft)
+		for dependent := range set {
+			if !removeSet.Contains(dependent) {
+				log.Fatalf("cannot remove %q without also removing %q, which depends on it", ft, dependent)
+			}
+		}
+		tags = append(tags, ft.OmitTag())
 	}
 	slices.Sort(tags)
 	tags = slices.Compact(tags)

cmd/omitsize/omitsize.go

@@ -10,56 +10,69 @@ import (
 	"flag"
 	"fmt"
 	"log"
+	"maps"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"slices"
 	"strconv"
 	"strings"
+	"sync"
 
 	"tailscale.com/feature/featuretags"
+	"tailscale.com/util/set"
 )
 
 var (
 	cacheDir = flag.String("cachedir", "", "if non-empty, use this directory to store cached size results to speed up subsequent runs. The tool does not consider the git status when deciding whether to use the cache. It's on you to nuke it between runs if the tree changed.")
-	features = flag.String("features", "", "comma-separated list of features to list in the table, with or without the ts_omit_ prefix. It may also contain a '+' sign(s) for ANDing features together. If empty, all omittable features are considered one at a time.")
+	features = flag.String("features", "", "comma-separated list of features to list in the table, without the ts_omit_ prefix. It may also contain a '+' sign(s) for ANDing features together. If empty, all omittable features are considered one at a time.")
 
 	showRemovals = flag.Bool("show-removals", false, "if true, show a table of sizes removing one feature at a time from the full set.")
 )
 
-func main() {
-	flag.Parse()
-
-	var all []string
-	var allOmittable []string
-
+// allOmittable returns the list of all build tags that remove features.
+var allOmittable = sync.OnceValue(func() []string {
+	var ret []string // all build tags that can be omitted
 	for k := range featuretags.Features {
 		if k.IsOmittable() {
-			allOmittable = append(allOmittable, k.OmitTag())
+			ret = append(ret, k.OmitTag())
 		}
 	}
+	slices.Sort(ret)
+	return ret
+})
+
+func main() {
+	flag.Parse()
+
+	// rows is a set (usually of size 1) of feature(s) to add/remove, without deps
+	// included at this point (as dep direction depends on whether we're adding or removing,
+	// so it's expanded later)
+	var rows []set.Set[featuretags.FeatureTag]
 
 	if *features == "" {
-		all = slices.Clone(allOmittable)
+		for _, k := range slices.Sorted(maps.Keys(featuretags.Features)) {
+			if k.IsOmittable() {
+				rows = append(rows, set.Of(k))
+			}
+		}
 	} else {
 		for v := range strings.SplitSeq(*features, ",") {
-			var withOmit []string
-			for v := range strings.SplitSeq(v, "+") {
-				if !strings.HasPrefix(v, "ts_omit_") {
-					v = "ts_omit_" + v
+			s := set.Set[featuretags.FeatureTag]{}
+			for fts := range strings.SplitSeq(v, "+") {
+				ft := featuretags.FeatureTag(fts)
+				if _, ok := featuretags.Features[ft]; !ok {
+					log.Fatalf("unknown feature %q", v)
 				}
-				withOmit = append(withOmit, v)
+				s.Add(ft)
 			}
-			all = append(all, strings.Join(withOmit, "+"))
+			rows = append(rows, s)
 		}
 	}
 
-	slices.Sort(all)
-	all = slices.Compact(all)
-
-	minD := measure("tailscaled", allOmittable...)
-	minC := measure("tailscale", allOmittable...)
-	minBoth := measure("tailscaled", append(slices.Clone(allOmittable), "ts_include_cli")...)
+	minD := measure("tailscaled", allOmittable()...)
+	minC := measure("tailscale", allOmittable()...)
+	minBoth := measure("tailscaled", append(slices.Clone(allOmittable()), "ts_include_cli")...)
 
 	if *showRemovals {
 		baseD := measure("tailscaled")
@@ -71,33 +84,108 @@ func main() {
 		fmt.Printf("%9s %9s %9s\n", "tailscaled", "tailscale", "combined (linux/amd64)")
 		fmt.Printf("%9d %9d %9d\n", baseD, baseC, baseBoth)
 
-		fmt.Printf("-%8d -%8d -%8d omit-all\n", baseD-minD, baseC-minC, baseBoth-minBoth)
+		fmt.Printf("-%8d -%8d -%8d .. remove *\n", baseD-minD, baseC-minC, baseBoth-minBoth)
 
-		for _, t := range all {
-			if strings.Contains(t, "+") {
-				log.Fatalf("TODO: make --show-removals support ANDed features like %q", t)
-			}
-			sizeD := measure("tailscaled", t)
-			sizeC := measure("tailscale", t)
-			sizeBoth := measure("tailscaled", append([]string{t}, "ts_include_cli")...)
+		for _, s := range rows {
+			title, tags := computeRemove(s)
+			sizeD := measure("tailscaled", tags...)
+			sizeC := measure("tailscale", tags...)
+			sizeBoth := measure("tailscaled", append(slices.Clone(tags), "ts_include_cli")...)
 			saveD := max(baseD-sizeD, 0)
 			saveC := max(baseC-sizeC, 0)
 			saveBoth := max(baseBoth-sizeBoth, 0)
-			fmt.Printf("-%8d -%8d -%8d %s\n", saveD, saveC, saveBoth, t)
+			fmt.Printf("-%8d -%8d -%8d .. remove %s\n", saveD, saveC, saveBoth, title)
+
 		}
 	}
 
-	fmt.Printf("\nStarting at a minimal binary and adding one feature back...\n")
+	fmt.Printf("\nStarting at a minimal binary and adding one feature back...\n\n")
 	fmt.Printf("%9s %9s %9s\n", "tailscaled", "tailscale", "combined (linux/amd64)")
 	fmt.Printf("%9d %9d %9d omitting everything\n", minD, minC, minBoth)
-	for _, t := range all {
-		tags := allExcept(allOmittable, strings.Split(t, "+"))
+	for _, s := range rows {
+		title, tags := computeAdd(s)
 		sizeD := measure("tailscaled", tags...)
 		sizeC := measure("tailscale", tags...)
 		sizeBoth := measure("tailscaled", append(tags, "ts_include_cli")...)
-		fmt.Printf("+%8d +%8d +%8d .. add %s\n", max(sizeD-minD, 0), max(sizeC-minC, 0), max(sizeBoth-minBoth, 0), strings.ReplaceAll(t, "ts_omit_", ""))
+
+		fmt.Printf("+%8d +%8d +%8d .. add %s\n", max(sizeD-minD, 0), max(sizeC-minC, 0), max(sizeBoth-minBoth, 0), title)
+	}
+
+}
+
+// computeAdd returns a human-readable title of a set of features and the build
+// tags to use to add that set of features to a minimal binary, including their
+// feature dependencies.
+func computeAdd(s set.Set[featuretags.FeatureTag]) (title string, tags []string) {
+	allSet := set.Set[featuretags.FeatureTag]{} // s + all their outbound dependencies
+	var explicitSorted []string                 // string versions of s, sorted
+	for ft := range s {
+		allSet.AddSet(featuretags.Requires(ft))
+		if ft.IsOmittable() {
+			explicitSorted = append(explicitSorted, string(ft))
+		}
+	}
+	slices.Sort(explicitSorted)
+
+	var removeTags []string
+	for ft := range allSet {
+		if ft.IsOmittable() {
+			removeTags = append(removeTags, ft.OmitTag())
+		}
+	}
+
+	var titleBuf strings.Builder
+	titleBuf.WriteString(strings.Join(explicitSorted, "+"))
+	var and []string
+	for ft := range allSet {
+		if !s.Contains(ft) {
+			and = append(and, string(ft))
+		}
+	}
+	if len(and) > 0 {
+		slices.Sort(and)
+		fmt.Fprintf(&titleBuf, " (and %s)", strings.Join(and, "+"))
+	}
+	tags = allExcept(allOmittable(), removeTags)
+	return titleBuf.String(), tags
+}
+
+// computeRemove returns a human-readable title of a set of features and the build
+// tags to use to remove that set of features from a full binary, including removing
+// any features that depend on features in the provided set.
+func computeRemove(s set.Set[featuretags.FeatureTag]) (title string, tags []string) {
+	allSet := set.Set[featuretags.FeatureTag]{} // s + all their inbound dependencies
+	var explicitSorted []string                 // string versions of s, sorted
+	for ft := range s {
+		allSet.AddSet(featuretags.RequiredBy(ft))
+		if ft.IsOmittable() {
+			explicitSorted = append(explicitSorted, string(ft))
+		}
+	}
+	slices.Sort(explicitSorted)
+
+	var removeTags []string
+	for ft := range allSet {
+		if ft.IsOmittable() {
+			removeTags = append(removeTags, ft.OmitTag())
+		}
+	}
+
+	var titleBuf strings.Builder
+	titleBuf.WriteString(strings.Join(explicitSorted, "+"))
+
+	var and []string
+	for ft := range allSet {
+		if !s.Contains(ft) {
+			and = append(and, string(ft))
+		}
+	}
+	if len(and) > 0 {
+		slices.Sort(and)
+		fmt.Fprintf(&titleBuf, " (and %s)", strings.Join(and, "+"))
 	}
 
+	return titleBuf.String(), removeTags
 }
 
 func allExcept(all, omit []string) []string {
@@ -120,7 +208,7 @@ func measure(bin string, tags ...string) int64 {
 		}
 	}
 
-	cmd := exec.Command("go", "build", "-tags", strings.Join(tags, ","), "-o", "tmpbin", "./cmd/"+bin)
+	cmd := exec.Command("go", "build", "-trimpath", "-ldflags=-w -s", "-tags", strings.Join(tags, ","), "-o", "tmpbin", "./cmd/"+bin)
 	log.Printf("# Measuring %v", cmd.Args)
 	cmd.Env = append(os.Environ(), "CGO_ENABLED=0", "GOOS=linux", "GOARCH=amd64")
 	out, err := cmd.CombinedOutput()

feature/featuretags/featuretags.go

@@ -4,6 +4,8 @@
 // The featuretags package is a registry of all the ts_omit-able build tags.
 package featuretags
 
+import "tailscale.com/util/set"
+
 // CLI is a special feature in the [Features] map that works opposite
 // from the others: it is opt-in, rather than opt-out, having a different
 // build tag format.
@@ -32,37 +34,90 @@ func (ft FeatureTag) OmitTag() string {
 	return "ts_omit_" + string(ft)
 }
 
+// Requires returns the set of features that must be included to
+// use the given feature, including the provided feature itself.
+func Requires(ft FeatureTag) set.Set[FeatureTag] {
+	s := set.Set[FeatureTag]{}
+	var add func(FeatureTag)
+	add = func(ft FeatureTag) {
+		if !ft.IsOmittable() {
+			return
+		}
+		s.Add(ft)
+		for _, dep := range Features[ft].Deps {
+			add(dep)
+		}
+	}
+	add(ft)
+	return s
+}
+
+// RequiredBy is the inverse of Requires: it returns the set of features that
+// depend on the given feature (directly or indirectly), including the feature
+// itself.
+func RequiredBy(ft FeatureTag) set.Set[FeatureTag] {
+	s := set.Set[FeatureTag]{}
+	for f := range Features {
+		if featureDependsOn(f, ft) {
+			s.Add(f)
+		}
+	}
+	return s
+}
+
+// featureDependsOn reports whether feature a (directly or indirectly) depends on b.
+// It returns true if a == b.
+func featureDependsOn(a, b FeatureTag) bool {
+	if a == b {
+		return true
+	}
+	for _, dep := range Features[a].Deps {
+		if featureDependsOn(dep, b) {
+			return true
+		}
+	}
+	return false
+}
+
 // FeatureMeta describes a modular feature that can be conditionally linked into
 // the binary.
 type FeatureMeta struct {
-	Sym  string // exported Go symbol for boolean const
-	Desc string // human-readable description
+	Sym  string       // exported Go symbol for boolean const
+	Desc string       // human-readable description
+	Deps []FeatureTag // other features this feature requires
 }
 
 // Features are the known Tailscale features that can be selectively included or
 // excluded via build tags, and a description of each.
 var Features = map[FeatureTag]FeatureMeta{
-	"acme":             {"ACME", "ACME TLS certificate management"},
-	"aws":              {"AWS", "AWS integration"},
-	"bird":             {"Bird", "Bird BGP integration"},
-	"capture":          {"Capture", "Packet capture"},
-	"cli":              {"CLI", "embed the CLI into the tailscaled binary"},
-	"completion":       {"Completion", "CLI shell completion"},
-	"debugeventbus":    {"DebugEventBus", "eventbus debug support"},
-	"debugportmapper":  {"DebugPortMapper", "portmapper debug support"},
-	"desktop_sessions": {"DesktopSessions", "Desktop sessions support"},
-	"drive":            {"Drive", "Tailscale Drive (file server) support"},
-	"kube":             {"Kube", "Kubernetes integration"},
-	"portmapper":       {"PortMapper", "NAT-PMP/PCP/UPnP port mapping support"},
-	"relayserver":      {"RelayServer", "Relay server"},
-	"serve":            {"Serve", "Serve and Funnel support"},
-	"ssh":              {"SSH", "Tailscale SSH support"},
-	"syspolicy":        {"SystemPolicy", "System policy configuration (MDM) support"},
-	"systray":          {"SysTray", "Linux system tray"},
-	"taildrop":         {"Taildrop", "Taildrop (file sending) support"},
-	"tailnetlock":      {"TailnetLock", "Tailnet Lock support"},
-	"tap":              {"Tap", "Experimental Layer 2 (ethernet) support"},
-	"tpm":              {"TPM", "TPM support"},
-	"wakeonlan":        {"WakeOnLAN", "Wake-on-LAN support"},
-	"webclient":        {"WebClient", "Web client support"},
+	"acme":          {"ACME", "ACME TLS certificate management", nil},
+	"aws":           {"AWS", "AWS integration", nil},
+	"bird":          {"Bird", "Bird BGP integration", nil},
+	"capture":       {"Capture", "Packet capture", nil},
+	"cli":           {"CLI", "embed the CLI into the tailscaled binary", nil},
+	"completion":    {"Completion", "CLI shell completion", nil},
+	"debugeventbus": {"DebugEventBus", "eventbus debug support", nil},
+	"debugportmapper": {
+		Sym:  "DebugPortMapper",
+		Desc: "portmapper debug support",
+		Deps: []FeatureTag{"portmapper"},
+	},
+	"desktop_sessions": {"DesktopSessions", "Desktop sessions support", nil},
+	"drive":            {"Drive", "Tailscale Drive (file server) support", nil},
+	"kube":             {"Kube", "Kubernetes integration", nil},
+	"portmapper":       {"PortMapper", "NAT-PMP/PCP/UPnP port mapping support", nil},
+	"relayserver":      {"RelayServer", "Relay server", nil},
+	"serve":            {"Serve", "Serve and Funnel support", nil},
+	"ssh":              {"SSH", "Tailscale SSH support", nil},
+	"syspolicy":        {"SystemPolicy", "System policy configuration (MDM) support", nil},
+	"systray":          {"SysTray", "Linux system tray", nil},
+	"taildrop":         {"Taildrop", "Taildrop (file sending) support", nil},
+	"tailnetlock":      {"TailnetLock", "Tailnet Lock support", nil},
+	"tap":              {"Tap", "Experimental Layer 2 (ethernet) support", nil},
+	"tpm":              {"TPM", "TPM support", nil},
+	"wakeonlan":        {"WakeOnLAN", "Wake-on-LAN support", nil},
+	"webclient": {
+		Sym: "WebClient", Desc: "Web client support",
+		Deps: []FeatureTag{"serve"},
+	},
 }

feature/featuretags/featuretags_test.go

@@ -0,0 +1,81 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package featuretags
+
+import (
+	"maps"
+	"slices"
+	"testing"
+
+	"tailscale.com/util/set"
+)
+
+func TestRequires(t *testing.T) {
+	for tag, meta := range Features {
+		for _, dep := range meta.Deps {
+			if _, ok := Features[dep]; !ok {
+				t.Errorf("feature %q has unknown dependency %q", tag, dep)
+			}
+		}
+
+		// And indirectly check for cycles. If there were a cycle,
+		// this would infinitely loop.
+		deps := Requires(tag)
+		t.Logf("deps of %q: %v", tag, slices.Sorted(maps.Keys(deps)))
+	}
+}
+
+func TestDepSet(t *testing.T) {
+	var setOf = set.Of[FeatureTag]
+	tests := []struct {
+		in   FeatureTag
+		want set.Set[FeatureTag]
+	}{
+		{
+			in:   "drive",
+			want: setOf("drive"),
+		},
+		{
+			in:   "serve",
+			want: setOf("serve"),
+		},
+		{
+			in:   "webclient",
+			want: setOf("webclient", "serve"),
+		},
+	}
+	for _, tt := range tests {
+		got := Requires(tt.in)
+		if !maps.Equal(got, tt.want) {
+			t.Errorf("DepSet(%q) = %v, want %v", tt.in, got, tt.want)
+		}
+	}
+}
+
+func TestRequiredBy(t *testing.T) {
+	var setOf = set.Of[FeatureTag]
+	tests := []struct {
+		in   FeatureTag
+		want set.Set[FeatureTag]
+	}{
+		{
+			in:   "drive",
+			want: setOf("drive"),
+		},
+		{
+			in:   "webclient",
+			want: setOf("webclient"),
+		},
+		{
+			in:   "serve",
+			want: setOf("webclient", "serve"),
+		},
+	}
+	for _, tt := range tests {
+		got := RequiredBy(tt.in)
+		if !maps.Equal(got, tt.want) {
+			t.Errorf("FeaturesWhichDependOn(%q) = %v, want %v", tt.in, got, tt.want)
+		}
+	}
+}