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

Multiple fixes for ONBUILD behavior #5397

Merged
merged 3 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1015,6 +1016,7 @@ type dispatchState struct {
unregistered bool
stageName string
cmdIndex int
cmdIsOnBuild bool
cmdTotal int
prefixPlatform bool
outline outlineCapture
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -1134,6 +1141,7 @@ func initOnBuildTriggers(d *dispatchState, triggers []string, allDispatchStates
}
}
d.commands = append(commands, d.commands...)
d.cmdTotal += len(commands)

return hasNewDeps, nil
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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) {
Expand Down
14 changes: 13 additions & 1 deletion frontend/dockerfile/parser/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
Loading