gobco icon indicating copy to clipboard operation
gobco copied to clipboard

source directory corruption

Open KantarBruceAdams opened this issue 6 years ago • 5 comments

Hi, I don't know how but gobco corrupted my original source replacing branches with calls to gobcocover. The first I knew of this was when "go build" started to fail with multiple:

undefined: gobcoCover

I appreciate that this is how it works under the hood but there should be no circumstance in which it can replace the original source. I have my source in source control so the damage was limited

  •   if awaiter == nil {
    
  •   if gobcoCover(9, awaiter == nil) {
    

The modified source should be in a temporary directory and never replace the original. I don't know quite how to reproduce this. I suspect it was by pressing Ctrl-C at an inopportune moment.

KantarBruceAdams avatar Jul 10 '19 17:07 KantarBruceAdams

I am using rillig's fork - https://github.com/rillig/gobco which unfortunately does not have an issue tracker configured. I don't know if this original version has the same issue.

KantarBruceAdams avatar Jul 10 '19 17:07 KantarBruceAdams

I believe this version may have the same defect based on:

func instrument(name string) {
...
	cmd := exec.Command("mv", name, name+".tmp")
	err = cmd.Run()
	check(err)
	fd, err := os.Create(name)

It is moving the original version to a temporary and replacing it. I note that I could not find any temporary files afterwards. The defect in Rillig's version is slightly more obvious:

// instrument reads the given file or directory and instruments the code for
// branch coverage. It writes the instrumented code back into the same files.
func (i *instrumenter) instrument(arg string, isDir bool) {

...
			// FIXME: Renaming files is not the job of the instrumenter.
			i.check(os.Rename(filename, filename+".gobco.tmp"))

			fd, err := os.Create(filename)

KantarBruceAdams avatar Jul 11 '19 10:07 KantarBruceAdams

I can confirm press ctrl-C while gobco is running the tests (e.g. during a long running test) is sufficient to reproduce this issue.

The code should write its modified a copies of the source in a temporary directory and run go test on that. I recommend to anyone wishing to use gobco (which obviously works well) to copy there source manually before running it as a workaround. Eg.


cp -rf src src4coverage
cd src4coverage
gobco > coverage.log

KantarBruceAdams avatar Jul 11 '19 10:07 KantarBruceAdams

@KantarBruceAdams

I just added the issue tracker to my fork. Thank you for notifying me.

Regarding the original issue. I know that it's currently bad, and this unnecessary in-place file modification already wasted some of my own time since I cannot continue editing while the branch coverage is running.

I already added lots of TODOs to main.go in the function mainNew, which is supposed to replace the current main once it is finished. I'll let you know when I got it running.

In the meantime, you can rename *.gobco.tmp back to the original files. And if you want to tackle the root cause, feel free to implement mainNew yourself. Otherwise you have to wait until I get to do it.

rillig avatar Jul 11 '19 17:07 rillig

@KantarBruceAdams Thank you for raising this issue and making me fix it. It feels much better that gobco does all its work in a temporary directory now and I can press Ctrl+C at any time without any danger.

rillig avatar Jul 30 '19 19:07 rillig