Merge pull request moby#5397 from tonistiigi/onbuild-fixes

Multiple fixes for ONBUILD behavior

Author: tonistiigi

frontend/dockerfile/dockerfile2llb/convert.go

@@ -602,7 +602,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 
 			if len(d.image.Config.OnBuild) > 0 {
 				if b, err := initOnBuildTriggers(d, d.image.Config.OnBuild, allDispatchStates); err != nil {
-					return nil, err
+					return nil, parser.SetLocation(err, d.stage.Location)
 				} else if b {
 					newDeps = true
 				}
@@ -788,7 +788,7 @@ func toCommand(ic instructions.Command, allDispatchStates *dispatchStates) (comm
 				stn, ok = allDispatchStates.findStateByName(c.From)
 				if !ok {
 					stn = &dispatchState{
-						stage:        instructions.Stage{BaseName: c.From, Location: ic.Location()},
+						stage:        instructions.Stage{BaseName: c.From, Location: c.Location()},
 						deps:         make(map[*dispatchState]instructions.Command),
 						paths:        make(map[string]struct{}),
 						unregistered: true,
@@ -860,6 +860,7 @@ func (e *envsFromState) Keys() []string {
 }
 
 func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
+	d.cmdIsOnBuild = cmd.isOnBuild
 	var err error
 	// ARG command value could be ignored, so defer handling the expansion error
 	_, isArg := cmd.Command.(*instructions.ArgCommand)
@@ -1015,6 +1016,7 @@ type dispatchState struct {
 	unregistered   bool
 	stageName      string
 	cmdIndex       int
+	cmdIsOnBuild   bool
 	cmdTotal       int
 	prefixPlatform bool
 	outline        outlineCapture
@@ -1093,7 +1095,8 @@ func (dss *dispatchStates) lastTarget() *dispatchState {
 
 type command struct {
 	instructions.Command
-	sources []*dispatchState
+	sources   []*dispatchState
+	isOnBuild bool
 }
 
 // initOnBuildTriggers initializes the onbuild triggers and creates the commands and dependecies for them.
@@ -1110,6 +1113,9 @@ func initOnBuildTriggers(d *dispatchState, triggers []string, allDispatchStates
 		if len(ast.AST.Children) != 1 {
 			return false, errors.New("onbuild trigger should be a single expression")
 		}
+		node := ast.AST.Children[0]
+		// reset the location to the onbuild trigger
+		node.StartLine, node.EndLine = rangeStartEnd(d.stage.Location)
 		ic, err := instructions.ParseCommand(ast.AST.Children[0])
 		if err != nil {
 			return false, err
@@ -1118,6 +1124,7 @@ func initOnBuildTriggers(d *dispatchState, triggers []string, allDispatchStates
 		if err != nil {
 			return false, err
 		}
+		cmd.isOnBuild = true
 		if len(cmd.sources) > 0 {
 			hasNewDeps = true
 		}
@@ -1134,6 +1141,7 @@ func initOnBuildTriggers(d *dispatchState, triggers []string, allDispatchStates
 		}
 	}
 	d.commands = append(commands, d.commands...)
+	d.cmdTotal += len(commands)
 
 	return hasNewDeps, nil
 }
@@ -2065,6 +2073,9 @@ func prefixCommand(ds *dispatchState, str string, prefixPlatform bool, platform
 	}
 	ds.cmdIndex++
 	out += fmt.Sprintf("%*d/%d] ", int(1+math.Log10(float64(ds.cmdTotal))), ds.cmdIndex, ds.cmdTotal)
+	if ds.cmdIsOnBuild {
+		out += "ONBUILD "
+	}
 	return out + str
 }
 
@@ -2509,6 +2520,23 @@ func buildMetaArgs(args *llb.EnvList, shlex *shell.Lex, argCommands []instructio
 	return args, allArgs, nil
 }
 
+func rangeStartEnd(r []parser.Range) (int, int) {
+	if len(r) == 0 {
+		return 0, 0
+	}
+	start := math.MaxInt32
+	end := 0
+	for _, rng := range r {
+		if rng.Start.Line < start {
+			start = rng.Start.Line
+		}
+		if rng.End.Line > end {
+			end = rng.End.Line
+		}
+	}
+	return start, end
+}
+
 type emptyEnvs struct{}
 
 func (emptyEnvs) Get(string) (string, bool) {

frontend/dockerfile/parser/errors.go

@@ -34,12 +34,24 @@ func withLocation(err error, start, end int) error {
 
 // WithLocation extends an error with a source code location
 func WithLocation(err error, location []Range) error {
+	return setLocation(err, location, true)
+}
+
+func SetLocation(err error, location []Range) error {
+	return setLocation(err, location, false)
+}
+
+func setLocation(err error, location []Range, add bool) error {
 	if err == nil {
 		return nil
 	}
 	var el *ErrorLocation
 	if errors.As(err, &el) {
-		el.Locations = append(el.Locations, location)
+		if add {
+			el.Locations = append(el.Locations, location)
+		} else {
+			el.Locations = [][]Range{location}
+		}
 		return err
 	}
 	return stack.Enable(&ErrorLocation{