[GH-ISSUE #40] HostsPerLine is destructive

kerem commented

2026-03-02 05:08:07 +03:00

Owner

Originally created by @tomjn on GitHub (Dec 4, 2022).
Original GitHub issue: https://github.com/goodhosts/hostsfile/issues/40

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 https://github.com/goodhosts/vagrant/issues/51

Originally created by @tomjn on GitHub (Dec 4, 2022). Original GitHub issue: https://github.com/goodhosts/hostsfile/issues/40 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 https://github.com/goodhosts/vagrant/issues/51

kerem closed this issue

2026-03-02 05:08:07 +03:00

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@jd4u commented on GitHub (Dec 4, 2022):

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.

@jd4u commented on GitHub (Dec 4, 2022): The issue seems to be in goodhosts/cli project. With or without "--clean" flag, cli is performing clean process. See [Issue Comment](https://github.com/goodhosts/vagrant/issues/51#issuecomment-1336280251) The cli project seems to be handling bool value incorrectly. Explicitly setting default bool value may help.

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@tomjn commented on GitHub (Dec 4, 2022):

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 commented on GitHub (Dec 4, 2022): 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

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@tomjn commented on GitHub (Dec 4, 2022):

@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 commented on GitHub (Dec 4, 2022): @luthermonson is it possible that this is all because of the Ip de-duplication at the top of that function? ```go 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() ```

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@tomjn commented on GitHub (Dec 4, 2022):

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?

@tomjn commented on GitHub (Dec 4, 2022): 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?

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@jd4u commented on GitHub (Dec 4, 2022):

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 commented on GitHub (Dec 4, 2022): 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.

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@jd4u commented on GitHub (Dec 4, 2022):

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.

@jd4u commented on GitHub (Dec 4, 2022): 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.

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@rfay commented on GitHub (Apr 6, 2023):

Comments are stripped regardless of the cli.

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

@rfay commented on GitHub (Apr 6, 2023): Comments are stripped regardless of the cli. This is easy to demonstrate with https://github.com/rfay/goodhostsbug

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@rfay commented on GitHub (Apr 6, 2023):

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 commented on GitHub (Apr 6, 2023): 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).

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@rfay commented on GitHub (Oct 27, 2023):

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

@rfay commented on GitHub (Oct 27, 2023): I've seen some good maintenance going on in this area lately, thanks! Has this one been addressed yet?

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@luthermonson commented on GitHub (Oct 27, 2023):

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

@luthermonson commented on GitHub (Oct 27, 2023): 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

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@rfay commented on GitHub (Oct 27, 2023):

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

@rfay commented on GitHub (Oct 27, 2023): 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 ```

kerem commented

2026-03-02 05:08:08 +03:00

Author

Owner

@luthermonson commented on GitHub (Oct 28, 2023):

im sorry i wasted your time... I forgot I hadn't released the changes here https://github.com/goodhosts/hostsfile/pull/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

@luthermonson commented on GitHub (Oct 28, 2023): im sorry i wasted your time... I forgot I hadn't released the changes here https://github.com/goodhosts/hostsfile/pull/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 ```

kerem commented

2026-03-02 05:08:09 +03:00

Author

Owner

@rfay commented on GitHub (Oct 28, 2023):

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

@rfay commented on GitHub (Oct 28, 2023): Yay, it's working great! This issue can be closed (at least about the comments), thanks so much!

kerem commented

2026-03-02 05:08:09 +03:00

Author

Owner

@luthermonson commented on GitHub (Oct 28, 2023):

My pleasure! Glad it's working

@luthermonson commented on GitHub (Oct 28, 2023): My pleasure! Glad it's working

kerem commented

2026-03-02 05:08:09 +03:00

Author

Owner

@rfay commented on GitHub (Oct 28, 2023):

Thanks so much for your maintenance of this! Reopened

https://github.com/ddev/ddev/pull/4805

@rfay commented on GitHub (Oct 28, 2023): Thanks so much for your maintenance of this! Reopened * https://github.com/ddev/ddev/pull/4805

kerem referenced this issue

2026-03-02 05:08:13 +03:00

[PR #14] [MERGED] fix panic in combineIp #27

Rows
Columns

[GH-ISSUE #40] HostsPerLine is destructive #14