HostsPerLine is destructive

[tomjn]

The HostsPerLine function is destructive, and does a number of unexpected things:

• it de-dupes which is the responsibility of a different function
• it's stripping out comments

I believe this is related to goodhosts/vagrant#51

[jd4u]

The issue seems to be in goodhosts/cli project. With or without "--clean" flag, cli is performing clean process. See Issue Comment

The cli project seems to be handling bool value incorrectly. Explicitly setting default bool value may help.

[tomjn]

Note that I cannot reproduce this on MacOS, this is very likely Windows related.

We need to confirm what the command is doing to the hosts file in addition to stripping out comments to eliminate options, my current theory is that this has nothing to do with the clean parameters, but due to the attempt to workaround a Windows bug where more than 8 hosts on a line breaks the hosts file.

We can confirm this if we had examples that can be used to see if it's sorting the hosts file, merging lines that were previously separate, etc

[tomjn]

@luthermonson is it possible that this is all because of the Ip de-duplication at the top of that function?

func (h *Hosts) HostsPerLine(count int) {
	// restacks everything into 1 ip again so we can do the split, do this even if count is -1 so it can reset the slice
	h.RemoveDuplicateIps()

[tomjn]

For context @jd4u is reporting that comments were stripped from their hosts file on use regardless of wether the clean parameter was specified or not. They're a WIndows user so HostsPerLine would be running to cap the lines at 8 hosts, is the RemoveDuplicateIps really necessary?

[jd4u]

Yes @tomjn, you may be pointing right place.

• RemoveDuplicateIps is called within Clean along with HostsPerLine
• Clean function does Sort before making HostsPerLine.
• RemoveDuplicateIps is called from HostsPerLine too.
• So even someone just calls HostsPerLine, RemoveDuplicateIps are called.

[jd4u]

Just for note,

• in VVV config, only 6 active sites present.
• And none of the IP had or have more than 4 hostnames in machine hosts file.

[rfay]

Comments are stripped regardless of the cli.

This is easy to demonstrate with https://github.com/rfay/goodhostsbug

[rfay]

One improvement here would be to respect "important" standard lines like "localhost". A longer-term improvement would be to add separate lines for goodhosts-managed lines (probably marked with a comment before and after).

[rfay]

I've seen some good maintenance going on in this area lately, thanks! Has this one been addressed yet?

[luthermonson]

ya all that stuff got refactored to index the lookup hashmaps better. i bet your example repo works against latest main, give it a check and report back findings and ill fix if still broke

[rfay]

No, it still makes a pretty big mess. Comments are mostly destroyed and misplaced. I updated https://github.com/rfay/goodhostsbug and its readme to show more how to demonstrate it.

Bottom line is that

##
# Host Database
#
# localhost is used to configure the loopback interface
# when the system is booting.  Do not change this entry.
##
127.0.0.1	localhost
255.255.255.255	broadcasthost
::1             localhost

gets transformed to

127.0.0.1 localhost xxx.ddev.site
255.255.255.255 broadcasthost
::1 localhost
  # Host Database

[luthermonson]

im sorry i wasted your time... I forgot I hadn't released the changes here #46

please try with v0.1.4 as with a test off latest main i got the following...

func Test_rfay(t *testing.T) {
	hosts := newMacOSXDefault()
	assert.Nil(t, hosts.Add("127.0.0.1", "xxx.ddev.site"))
	hosts.HostsPerLine(8)
	fmt.Println(hosts.String())
}

##
# Host Database
#
# localhost is used to configure the loopback interface
# when the system is booting.  Do not change this entry.
##
127.0.0.1 localhost xxx.ddev.site
255.255.255.255 broadcasthost
::1 localhost

[rfay]

Yay, it's working great! This issue can be closed (at least about the comments), thanks so much!

[luthermonson]

My pleasure! Glad it's working

[rfay]

Thanks so much for your maintenance of this! Reopened

• fix: implement limit on number of hosts per line in /etc/hosts because Safari has problems with long lines, fixes #2817 ddev/ddev#4805