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

ForceNoOutput causing graph duplication #350

Open
coryb opened this issue Mar 25, 2023 · 2 comments
Open

ForceNoOutput causing graph duplication #350

coryb opened this issue Mar 25, 2023 · 2 comments

Comments

@coryb
Copy link
Contributor

coryb commented Mar 25, 2023

When we run the second target from this HLB:

fs _first() {
	image "busybox"
	run "echo first" with option {
		mount scratch localCwd as first
	}
}

fs _second() {
	_first
	run "echo second" with option {
		mount first localCwd as second
	}
}

we will see something like:

#1 resolve image config for docker.io/library/busybox:latest
#1 DONE 0.4s

#2 docker-image://docker.io/library/busybox:latest
#2 resolve docker.io/library/busybox:latest
#2 resolve docker.io/library/busybox:latest 0.3s done
#2 CACHED

#3 /bin/sh -c 'echo first'
#3 CACHED

#4 /bin/sh -c 'echo first'
#4 0.270 first
#4 DONE 0.3s

#5 /bin/sh -c 'echo second'
#5 0.499 second
#5 DONE 0.5s

Note the echo first is evaluated twice, if we add noCache it will run echo first twice in two different containers. The duplication goes away when we remove this line

I am at a loss for how to fix this. It seems the first mount is getting evaluated twice once has ForceNoOutput applied and one has the Bind logic applied. I am not sure why the Bind logic would not be applied in both locations.

We should probably revert #347 and #348 unless anyone has ideas on how to work around this.

@hinshun
Copy link
Contributor

hinshun commented Mar 26, 2023

Can you reproduce this with just LLB? i.e. _first will be execSt.Root() whereas first will be execSt.GetMount(...).

@coryb
Copy link
Contributor Author

coryb commented Mar 26, 2023

Pretty sure this is just an an issue with how hlb generates the state. At least this does what I expect with LLB:

func main() {
	ctx := context.Background()
	c, err := client.New(context.Background(), "docker-container://buildkitd")
	if err != nil {
		panic(err)
	}
	defer c.Close()

	localCwd, _ := os.Getwd()
	execFirst := llb.Image("busybox").Run(
		llb.Shlex("echo first"),
		llb.AddMount(localCwd, llb.Scratch()),
	)
	_first := execFirst.Root()
	first := execFirst.GetMount(localCwd)

	second := _first.Run(
		llb.Shlex("echo second"),
		llb.AddMount(localCwd, first),
	).GetMount(localCwd)

	def, err := second.Marshal(ctx, llb.LinuxAmd64)
	if err != nil {
		panic(err)
	}

	ch := make(chan *client.SolveStatus)
	eg, ctx := errgroup.WithContext(ctx)
	eg.Go(func() error {
		_, err = c.Solve(ctx, def, client.SolveOpt{}, ch)
		return err
	})
	eg.Go(func() error {
		_, err = progressui.DisplaySolveStatus(context.TODO(), "", nil, os.Stdout, ch)
		return err
	})
	err = eg.Wait()
	if err != nil {
		panic(err)
	}
}

Running that I see:

#1 docker-image://docker.io/library/busybox:latest
#1 resolve docker.io/library/busybox:latest
#1 resolve docker.io/library/busybox:latest 0.3s done
#1 DONE 0.3s

#2 echo first
#2 CACHED

#3 echo second
#0 0.051 second
#3 DONE 0.1s

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

2 participants