Host files are stripped of comments on Windows

jd4u commented 1 year ago

goodhosts version 1.1.4 with vagrant 2.3.0 an vbox 6.1.38 (Windows 10)

Today upgraded from 1.1.1 to 1.1.4 and got the issue.

Issue: goodhosts auto-cleans hosts file and removes all comments

The issue made me loss multiple project mappings in hosts file that were commented.
In my case all the comments of hosts file is removed!!! and multiple records of each ip is created as single record....
Normally while changing user files, a copy must be created as a good practice...

Below is in the readme... That suggest goodhosts is not cleaning hosts file by default !!!! But not having the line in Vagrantfile, it cleans the hosts file. The plugin assumes total control of the hosts file of machine that runs it...!!!

================== Disable file hosts clean

If you want /etc/hosts file cleaned add in your VagrantFile:

config.goodhosts.disable_clean = false

==================

jd4u commented 1 year ago

This may be bug or wrong approach.

jd4u commented 1 year ago

The issue happens while halting process too. (this is without having config.goodhosts.disable_clean config in Vagrantfile)
Inline comments are kept. Comments starting with line are removed...
After adding config.goodhosts.disable_clean = false, the clean is happening in up and halt process
After changing config.goodhosts.disable_clean = true, the clean is happening in up and halt process

jd4u commented 1 year ago

In all of the below cases, hosts file is cleaned....

1] without adding config.goodhosts.disable_clean in Vagrantfile

[vagrant-goodhosts] Command for CLI: 'Start-Process 'C:/Users/Username/.vagrant.d/gems/2.7.6/gems/vagrant-goodhosts-1.1.4/lib/vagrant-goodhosts/bundle/cli.exe' -ArgumentList "add","192.168.50.4","vvv.test project1.test project2.test" -Verb RunAs'
[vagrant-goodhosts] Command for CLI: 'Start-Process 'C:/Users/Username/.vagrant.d/gems/2.7.6/gems/vagrant-goodhosts-1.1.4/lib/vagrant-goodhosts/bundle/cli.exe' -ArgumentList "remove","192.168.50.4","vvv.test project1.test project2.test" -Verb RunAs'

2] Adding config.goodhosts.disable_clean=true in Vagrantfile

[vagrant-goodhosts] Command for CLI: 'Start-Process 'C:/Users/Username/.vagrant.d/gems/2.7.6/gems/vagrant-goodhosts-1.1.4/lib/vagrant-goodhosts/bundle/cli.exe' -ArgumentList "add","192.168.50.4","vvv.test project1.test project2.test" -Verb RunAs'
[vagrant-goodhosts] Command for CLI: 'Start-Process 'C:/Users/Username/.vagrant.d/gems/2.7.6/gems/vagrant-goodhosts-1.1.4/lib/vagrant-goodhosts/bundle/cli.exe' -ArgumentList "remove","192.168.50.4","vvv.test project1.test project2.test" -Verb RunAs'

3] Adding config.goodhosts.disable_clean=false in Vagrantfile

[vagrant-goodhosts] Command for CLI: 'Start-Process 'C:/Users/Username/.vagrant.d/gems/2.7.6/gems/vagrant-goodhosts-1.1.4/lib/vagrant-goodhosts/bundle/cli.exe' -ArgumentList "add","--clean","192.168.50.4","vvv.test project1.test project2.test" -Verb RunAs'
[vagrant-goodhosts] Command for CLI: 'Start-Process 'C:/Users/Username/.vagrant.d/gems/2.7.6/gems/vagrant-goodhosts-1.1.4/lib/vagrant-goodhosts/bundle/cli.exe' -ArgumentList "remove","--clean","192.168.50.4","vvv.test project1.test project2.test" -Verb RunAs'

jd4u commented 1 year ago

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

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

jd4u commented 1 year ago

I wish @tomjn you find resolution soon, I see you have already found a sound pointer.

@tomjn , above all, I see management of hosts file on development machine, which is part of the OS/System components, should not be a task of any developer tools or library. Windows correctly provided UAC to disallow/prevent such actions.

Building capability in a lib is one aspect. But the enforcing a new design using that can cause lots of problem for users. ( Earlier approach of adding and removing one line to hosts file was working well and is enough!! )

VVV shall have full control within the development environment managed by it. But shall not touch the development machine components.

I hope you can understand, visualize and coordinate the design changes required.

JD

tomjn commented 1 year ago

This isn’t a VVV repo or project, and what you’re suggesting would cripple the workflows of thousands and mean no longer using this project or any equivalent. Please cease all discussion of VVV here.

On Sun, 4 Dec 2022 at 20:15, jd4u @.***> wrote:

I wish @tomjn https://github.com/tomjn you find resolution soon, I see you have already found a sound pointer.

@tomjn https://github.com/tomjn , above all, I see management of hosts file on development machine, which is part of the OS/System components, should not be a task of any developer tools or library. Windows correctly provided UAC to disallow/prevent such actions.

Building capability in a lib is one aspect. But the enforcing a new design using that can cause lots of problem for users. ( Earlier approach of adding and removing one line to hosts file was working well and is enough!! )

VVV shall have full control within the development environment managed by it. But shall not touch the development machine components.

I hope you can understand, visualize and coordinate the design changes required.

JD

— Reply to this email directly, view it on GitHub https://github.com/goodhosts/vagrant/issues/51#issuecomment-1336507160, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAOLZ5HFTLWGAFDWU2R6VTWLT3VBANCNFSM6AAAAAASS6THQU . You are receiving this because you were mentioned.Message ID: @.***>

Mte90 commented 1 year ago

The issue was fixed in VVV https://github.com/Varying-Vagrant-Vagrants/VVV/commit/bd2035302efe18534f9e8a8cdbf41c99e7392cc3

tomjn commented 1 year ago

@Mte90 it wasn't, the issue is not cleaning, it's a Windows specific issue and it happens regardless of wether --clean is present or not, re-read https://github.com/goodhosts/vagrant/issues/51#issuecomment-1336280251

tomjn commented 1 year ago

This is much more likely to be a part of the library used by this CLI: https://github.com/goodhosts/hostsfile/issues/40

Mte90 commented 1 year ago

I changed the ticket name as it wasn't clear to me what was the issue.

YouJiacheng commented 1 year ago

Yes. goodhosts will remove comments which might be useful for other application. But this should be an issue of goodhosts/hostsfile. I think this issue can be closed and tracked in https://github.com/goodhosts/hostsfile/issues/40

tomjn commented 1 year ago

we'd still need to perform a release with any changes made in the library and CLI so this needs to be kept open, and there's still the chance the issue is elsewhere

goodhosts / vagrant

Host files are stripped of comments on Windows #51