mwdd: Only save hosts file on change

https://phabricator.wikimedia.org/T292909

cmd/mwdd_hosts.go

@@ -36,7 +36,7 @@ var mwddHostsAddCmd = &cobra.Command{
 	Use:   "add",
 	Short: "Adds development environment hosts into your system hosts file (might need sudo)",
 	Run: func(cmd *cobra.Command, args []string) {
-		save := hosts.AddHosts(
+		changeResult := hosts.AddHosts(
 			append(
 				[]string{
 					// TODO generate these by reading the yml files?
@@ -49,20 +49,7 @@ var mwddHostsAddCmd = &cobra.Command{
 				mwdd.DefaultForUser().UsedHosts()...,
 			),
 		)
-		if save.Success {
-			fmt.Println("Hosts file " + save.WriteFile + " updated!")
-		} else {
-			fmt.Println("Could not save your hosts file.")
-			fmt.Println("You can return with sudo.")
-			fmt.Println("Or edit the hosts file yourself.")
-			fmt.Println("Temporary file: " + save.WriteFile)
-			fmt.Println("")
-			fmt.Println(save.Content)
-			// Hack around https://phabricator.wikimedia.org/T292909
-			if os.Getenv("MWCLI_CONTEXT_TEST") == "" {
-				os.Exit(1)
-			}
-		}
+		handleChangeResult(changeResult)
 	},
 }
 
@@ -70,24 +57,25 @@ var mwddHostsRemoveCmd = &cobra.Command{
 	Use:   "remove",
 	Short: "Removes development environment hosts from your system hosts file (might need sudo)",
 	Run: func(cmd *cobra.Command, args []string) {
-		save := hosts.RemoveHostsWithSuffix("mwdd.localhost")
-		if save.Success {
-			fmt.Println("Hosts file " + save.WriteFile + " updated!")
-		} else {
-			fmt.Println("Could not save your hosts file.")
-			fmt.Println("You can return with sudo.")
-			fmt.Println("Or edit the hosts file yourself.")
-			fmt.Println("Temporary file: " + save.WriteFile)
-			fmt.Println("")
-			fmt.Println(save.Content)
-			// Hack around https://phabricator.wikimedia.org/T292909
-			if os.Getenv("MWCLI_CONTEXT_TEST") == "" {
-				os.Exit(1)
-			}
-		}
+		handleChangeResult(hosts.RemoveHostsWithSuffix("mwdd.localhost"))
 	},
 }
 
+func handleChangeResult(result hosts.ChangeResult) {
+	if result.Success && result.Altered {
+		fmt.Println("Hosts file altered and updated: " + result.WriteFile)
+	} else if result.Altered {
+		fmt.Println("Wanted to alter your hosts file bu could not.")
+		fmt.Println("You can re-run this command with sudo.")
+		fmt.Println("Or edit the hosts file yourself.")
+		fmt.Println("Temporary file: " + result.WriteFile)
+		fmt.Println("")
+		fmt.Println(result.Content)
+	} else {
+		fmt.Println("No changes needed.")
+	}
+}
+
 var mwddHostsWritableCmd = &cobra.Command{
 	Use:   "writable",
 	Short: "Checks if you can write to the needed hosts file",

internal/util/hosts/hosts.go

@@ -30,15 +30,16 @@ var (
 	hostsTmpPrefix = "mwcli-hosts-"
 )
 
-/*Save result of saving the hosts file.*/
-type Save struct {
+/*ChangeResult result of changing the hosts file.*/
+type ChangeResult struct {
 	Success   bool
+	Altered   bool
 	Content   string
 	WriteFile string
 }
 
 /*AddHosts attempts to add requested hosts to the system hosts file, and gives you the new content, a tmp file and success.*/
-func AddHosts(toAdd []string) Save {
+func AddHosts(toAdd []string) ChangeResult {
 	hosts := hosts()
 	serviceIP := "127.0.0.1"
 
@@ -55,14 +56,14 @@ func AddHosts(toAdd []string) Save {
 	// TODO when the library supports it do ipv6 too https://github.com/txn2/txeh/issues/15
 	// hosts.AddHosts("::1", toAdd)
 
-	return save(hosts)
+	return finishChanges(hosts)
 }
 
 /*RemoveHostsWithSuffix attempts to remove all hosts with a suffix from the system hosts file, and gives you the new content, a tmp file and success.*/
-func RemoveHostsWithSuffix(hostSuffix string) Save {
+func RemoveHostsWithSuffix(hostSuffix string) ChangeResult {
 	hosts := hosts()
 	removeHostsWithSuffixFromLines(hostSuffix, hosts)
-	return save(hosts)
+	return finishChanges(hosts)
 }
 
 /*Writable is the hosts file writable.*/
@@ -113,7 +114,20 @@ func removeHostsWithSuffixFromLines(hostSuffix string, hosts *txeh.Hosts) *txeh.
 	return hosts
 }
 
-func save(hosts *txeh.Hosts) Save {
+func finishChanges(touchedHosts *txeh.Hosts) ChangeResult {
+	diskHosts := hosts()
+	if diskHosts.RenderHostsFile() != touchedHosts.RenderHostsFile() {
+		return save(touchedHosts)
+	}
+	return ChangeResult{
+		Success:   true,
+		Altered:   false,
+		Content:   diskHosts.RenderHostsFile(),
+		WriteFile: diskHosts.WriteFilePath,
+	}
+}
+
+func save(hosts *txeh.Hosts) ChangeResult {
 	err := hosts.Save()
 	if err != nil {
 		tmpFile := tmpFile()
@@ -121,15 +135,17 @@ func save(hosts *txeh.Hosts) Save {
 		if err != nil {
 			panic(err)
 		}
-		return Save{
+		return ChangeResult{
 			Success:   false,
+			Altered:   true,
 			Content:   hosts.RenderHostsFile(),
 			WriteFile: tmpFile,
 		}
 	}
 
-	return Save{
+	return ChangeResult{
 		Success:   true,
+		Altered:   true,
 		Content:   hosts.RenderHostsFile(),
 		WriteFile: hosts.WriteFilePath,
 	}

internal/util/hosts/hosts_test.go

@@ -50,7 +50,7 @@ func TestAddHosts(t *testing.T) {
 		startingContent string
 		name            string
 		args            args
-		want            Save
+		want            ChangeResult
 	}{
 		{
 			name:            "Empty: Add single: 1.mwcli.test",
@@ -58,8 +58,9 @@ func TestAddHosts(t *testing.T) {
 			args: args{
 				toAdd: []string{"1.mwcli.test"},
 			},
-			want: Save{
+			want: ChangeResult{
 				Success:   true,
+				Altered:   true,
 				Content:   "127.0.0.1        1.mwcli.test\n",
 				WriteFile: "",
 			},
@@ -70,8 +71,9 @@ func TestAddHosts(t *testing.T) {
 			args: args{
 				toAdd: []string{"1.mwcli.test", "2.mwcli.test"},
 			},
-			want: Save{
+			want: ChangeResult{
 				Success:   true,
+				Altered:   true,
 				Content:   "127.0.0.1        1.mwcli.test 2.mwcli.test\n",
 				WriteFile: "",
 			},
@@ -82,8 +84,9 @@ func TestAddHosts(t *testing.T) {
 			args: args{
 				toAdd: []string{"1.mwcli.test", "2.mwcli.test"},
 			},
-			want: Save{
+			want: ChangeResult{
 				Success:   true,
+				Altered:   true,
 				Content:   "127.0.0.1        iam.localhost 1.mwcli.test 2.mwcli.test\n",
 				WriteFile: "",
 			},
@@ -94,8 +97,9 @@ func TestAddHosts(t *testing.T) {
 			args: args{
 				toAdd: []string{"1.mwcli.test", "2.mwcli.test"},
 			},
-			want: Save{
+			want: ChangeResult{
 				Success:   true,
+				Altered:   true,
 				Content:   "123.123.111.111  iam.not.localhost\n127.0.0.1        1.mwcli.test 2.mwcli.test\n",
 				WriteFile: "",
 			},
@@ -125,7 +129,7 @@ func TestRemoveHostsWithSuffix(t *testing.T) {
 		startingContent string
 		name            string
 		args            args
-		want            Save
+		want            ChangeResult
 	}{
 		{
 			name:            "Remove mwcli.test suffix, resulting in same content",
@@ -133,8 +137,9 @@ func TestRemoveHostsWithSuffix(t *testing.T) {
 			args: args{
 				hostSuffix: "mwcli.test",
 			},
-			want: Save{
+			want: ChangeResult{
 				Success:   true,
+				Altered:   false,
 				Content:   singleLocalHost,
 				WriteFile: "",
 			},
@@ -145,8 +150,9 @@ func TestRemoveHostsWithSuffix(t *testing.T) {
 			args: args{
 				hostSuffix: "mwcli.test",
 			},
-			want: Save{
+			want: ChangeResult{
 				Success:   true,
+				Altered:   true,
 				Content:   "",
 				WriteFile: "",
 			},
@@ -157,8 +163,9 @@ func TestRemoveHostsWithSuffix(t *testing.T) {
 			args: args{
 				hostSuffix: "mwcli.test",
 			},
-			want: Save{
+			want: ChangeResult{
 				Success:   true,
+				Altered:   true,
 				Content:   singleLocalHost,
 				WriteFile: "",
 			},