Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Root package changes not being considered for tainting #2

Open
TomDeVito opened this issue Jun 26, 2018 · 4 comments
Open

Root package changes not being considered for tainting #2

TomDeVito opened this issue Jun 26, 2018 · 4 comments

Comments

@TomDeVito
Copy link

Hey, thanks for making this tool, I ran into a little snag:

If I have the following structure:

cmd/
    app1/
        main.go     <- imports package project
    app2/
         main.go
project.go          <-  package name:  project

and I make changes to project.go, app1 and app2 are not considered to be tainted. project.go is in the CWD, but seems to be ignored here

When I change the code to the following:

if dir := filepath.Dir(scanner.Text()); dir != "." {
	changedDirs[dir] = struct{}{}
} else {
	ctx := build.Default
	cwd, err := os.Getwd()
	if err != nil {
		log.Fatal(err)
	}
	p, err := ctx.ImportDir(cwd, build.FindOnly)
	if err != nil {
		log.Fatal(err)
	}
	changedDirs[p.ImportPath] = struct{}{}
}

I seem to get my desired outcome. I'm not sure if this the best way to handle this or not and was wondering what your thoughts are about this matter.

Thanks,

@kynrai
Copy link
Owner

kynrai commented Jun 28, 2018

Hello @TomDeVito ,

Thanks for filing this. I must admit i never considered this case as .go files are often not in the root when following the recommended go structure, however I can totally see many situations where it would be the case.

Ill run some tests but I suspect removing the explicit check for root packages would fix this, I think I originally put the check for packages in . as an optimisation or that it was causing all packages to be marked as tainted.

Ill have a play.

@kynrai
Copy link
Owner

kynrai commented Jun 28, 2018

Also apologies for the late reply @TomDeVito

@TomDeVito
Copy link
Author

No worries, thanks for responding. I'm following Ben Johnson's Standard Package Layout which has domain structs at the root level. Yea, I tried removing it, but didn't find it working properly until I was able to discover what package was contained in the cwd

@tbruyelle
Copy link

tbruyelle commented Jul 27, 2018

I had the same issue and @TomDeVito's patch fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants