Skip to content

Commit 7ddaacf

Browse files
nalindnalind
committed
When stages have duplicate nicknames, make the last one win
When multiple stages all have the same nickname, when we attempt to resolve a name to a stage, choose the latest one possible. Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
1 parent ee26131 commit 7ddaacf

7 files changed

Lines changed: 170 additions & 6 deletions

File tree

‎builder.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ var (
213213
type Stages []Stage
214214

215215
func (stages Stages) ByName(name string) (Stage, bool) {
216-
for _, stage := range stages {
216+
for _, stage := range slices.Backward(stages) {
217217
if stage.Name == name {
218218
return stage, true
219219
}
@@ -241,7 +241,7 @@ func (stages Stages) ByTarget(target string) (Stages, bool) {
241241
if len(target) == 0 {
242242
return stages, true
243243
}
244-
for i, stage := range stages {
244+
for i, stage := range slices.Backward(stages) {
245245
if stage.Name == target {
246246
return stages[i : i+1], true
247247
}
@@ -264,7 +264,7 @@ func (stages Stages) ThroughTarget(target string) (Stages, bool) {
264264
if len(target) == 0 {
265265
return stages, true
266266
}
267-
for i, stage := range stages {
267+
for i, stage := range slices.Backward(stages) {
268268
if stage.Name == target {
269269
return stages[0 : i+1], true
270270
}
@@ -284,7 +284,7 @@ func (stages Stages) ThroughTarget(target string) (Stages, bool) {
284284

285285
type Stage struct {
286286
Position int
287-
Name string
287+
Name string // may just be strconv.Itoa(Position), be sure to search from back to front
288288
Builder *Builder
289289
Node *parser.Node
290290
}

‎builder_test.go‎

Lines changed: 148 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,101 @@ func TestVolumeSet(t *testing.T) {
7272
}
7373
}
7474

75+
func TestByName(t *testing.T) {
76+
n, err := ParseFile("dockerclient/testdata/Dockerfile.target")
77+
if err != nil {
78+
t.Fatal(err)
79+
}
80+
stages, err := NewStages(n, NewBuilder(nil))
81+
if err != nil {
82+
t.Fatal(err)
83+
}
84+
if len(stages) != 4 {
85+
t.Fatalf("expected 4 stages, got %d", len(stages))
86+
}
87+
t.Logf("stages: %#v", stages)
88+
89+
stage1, found := stages.ByName("mytarget")
90+
if !found {
91+
t.Fatal("First target not found")
92+
}
93+
if stage1.Position != 1 {
94+
t.Fatalf("expected stage at position 1, got %d", stage1.Position)
95+
}
96+
t.Logf("stage1: %#v", stage1)
97+
98+
stage2, found := stages.ByName("mytarget2")
99+
if !found {
100+
t.Fatal("Second target not found")
101+
}
102+
if stage2.Position != 2 {
103+
t.Fatalf("expected stage at position 2, got %d", stage1.Position)
104+
}
105+
t.Logf("stage2: %#v", stage2)
106+
107+
stage3, found := stages.ByName("1")
108+
if !found {
109+
t.Fatal("Third target not found")
110+
}
111+
if stage3.Position != 1 {
112+
t.Fatalf("expected stage at position 1, got %d", stage3.Position)
113+
}
114+
t.Logf("stage3: %#v", stage3)
115+
assert.Equal(t, stage3, stage1)
116+
117+
stage4, found := stages.ByName("2")
118+
if !found {
119+
t.Fatal("Fourth target not found")
120+
}
121+
if stage4.Position != 2 {
122+
t.Fatalf("expected stage at position 2, got %d", stage4.Position)
123+
}
124+
t.Logf("stage4: %#v", stage4)
125+
assert.Equal(t, stage4, stage2)
126+
127+
stage5, found := stages.ByName("mytarget3")
128+
if !found {
129+
t.Fatal("Fifth target not found")
130+
}
131+
if stage5.Position != 3 {
132+
t.Fatalf("expected stage at position 3, got %d", stage5.Position)
133+
}
134+
t.Logf("stage5: %#v", stage5)
135+
136+
stage6, found := stages.ByName("3")
137+
if !found {
138+
t.Fatal("Sixth target not found")
139+
}
140+
if stage6.Position != 3 {
141+
t.Fatalf("expected stage at position 3, got %d", stage6.Position)
142+
}
143+
t.Logf("stage6: %#v", stage6)
144+
assert.Equal(t, stage6, stage5)
145+
146+
n, err = ParseFile("dockerclient/testdata/Dockerfile.target")
147+
if err != nil {
148+
t.Fatal(err)
149+
}
150+
stages, err = NewStages(n, NewBuilder(map[string]string{"TARGET3": "mytarget"}))
151+
if err != nil {
152+
t.Fatal(err)
153+
}
154+
if len(stages) != 4 {
155+
t.Fatalf("expected 4 stage, got %d", len(stages))
156+
}
157+
t.Logf("stages: %#v", stages)
158+
159+
stage7, found := stages.ByName("mytarget")
160+
if !found {
161+
t.Fatal("Seventh target not found")
162+
}
163+
if stage7.Position != 3 {
164+
t.Fatalf("expected stage at position 3, got %d", stage7.Position)
165+
}
166+
t.Logf("stage7: %#v", stage7)
167+
168+
}
169+
75170
func TestByTarget(t *testing.T) {
76171
n, err := ParseFile("dockerclient/testdata/Dockerfile.target")
77172
if err != nil {
@@ -93,6 +188,9 @@ func TestByTarget(t *testing.T) {
93188
if len(stages1) != 1 {
94189
t.Fatalf("expected 1 stages, got %d", len(stages1))
95190
}
191+
if stages1[0].Position != 1 {
192+
t.Fatalf("expected stage at position 1, got %d", stages1[0].Position)
193+
}
96194
t.Logf("stages1: %#v", stages1)
97195

98196
stages2, found := stages.ByTarget("mytarget2")
@@ -138,10 +236,36 @@ func TestByTarget(t *testing.T) {
138236
t.Fatal("Sixth target not found")
139237
}
140238
if len(stages6) != 1 {
141-
t.Fatalf("expected 1 stages, got %d", len(stages4))
239+
t.Fatalf("expected 1 stages, got %d", len(stages6))
142240
}
143241
t.Logf("stages6: %#v", stages6)
144242
assert.Equal(t, stages6, stages5)
243+
244+
n, err = ParseFile("dockerclient/testdata/Dockerfile.target")
245+
if err != nil {
246+
t.Fatal(err)
247+
}
248+
stages, err = NewStages(n, NewBuilder(map[string]string{"TARGET3": "mytarget"}))
249+
if err != nil {
250+
t.Fatal(err)
251+
}
252+
if len(stages) != 4 {
253+
t.Fatalf("expected 4 stages, got %d", len(stages))
254+
}
255+
t.Logf("stages: %#v", stages)
256+
257+
stages7, found := stages.ByTarget("mytarget")
258+
if !found {
259+
t.Fatal("Seventh target not found")
260+
}
261+
if len(stages7) != 1 {
262+
t.Fatalf("expected 1 stages, got %d", len(stages7))
263+
}
264+
if stages7[0].Position != 3 {
265+
t.Fatalf("expected stage at position 3, got %d", stages7[0].Position)
266+
}
267+
t.Logf("stages7: %#v", stages7)
268+
145269
}
146270

147271
func TestThroughTarget(t *testing.T) {
@@ -214,6 +338,29 @@ func TestThroughTarget(t *testing.T) {
214338
}
215339
t.Logf("stages6: %#v", stages6)
216340
assert.Equal(t, stages6, stages5)
341+
342+
n, err = ParseFile("dockerclient/testdata/Dockerfile.target")
343+
if err != nil {
344+
t.Fatal(err)
345+
}
346+
347+
stages, err = NewStages(n, NewBuilder(map[string]string{"TARGET3": "mytarget"}))
348+
if err != nil {
349+
t.Fatal(err)
350+
}
351+
if len(stages) != 4 {
352+
t.Fatalf("expected 4 stages, got %d", len(stages))
353+
}
354+
t.Logf("stages: %#v", stages)
355+
356+
stages7, found := stages.ThroughTarget("mytarget")
357+
if !found {
358+
t.Fatal("Seventh target not found")
359+
}
360+
if len(stages7) != 4 {
361+
t.Fatalf("expected 4 stages, got %d", len(stages7))
362+
}
363+
t.Logf("stages7: %#v", stages7)
217364
}
218365

219366
func TestMultiStageParse(t *testing.T) {

‎dockerclient/client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ func (e *ClientExecutor) Stages(b *imagebuilder.Builder, stages imagebuilder.Sta
195195
return nil, fmt.Errorf("the --after flag in FROM is not supported by the dockerclient executor")
196196
}
197197
if prereq := e.Named[from]; prereq != nil {
198-
b, ok := stages.ByName(from)
198+
b, ok := stages[:i].ByName(from)
199199
if !ok {
200200
return nil, fmt.Errorf("error: Unable to find stage %s builder", from)
201201
}

‎dockerclient/conformance_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,12 @@ func TestConformanceInternal(t *testing.T) {
504504
Version: docker.BuilderBuildKit,
505505
Dockerfile: "testdata/builtins/Dockerfile.margs",
506506
},
507+
{
508+
Name: "duplicate-stage",
509+
Version: docker.BuilderBuildKit,
510+
ContextDir: "testdata/multistage",
511+
Dockerfile: "Dockerfile.dupstage",
512+
},
507513
{
508514
Name: "header-builtin",
509515
Version: docker.BuilderBuildKit,

‎dockerclient/testdata/multistage/Dockerfile.dupstage‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
FROM mirror.gcr.io/busybox AS stage0
2+
COPY --chown=0:0 stage0.txt /stage.txt
3+
LABEL x="really stage 0"
4+
5+
FROM mirror.gcr.io/busybox AS stage0
6+
COPY --chown=0:0 stage1.txt /stage.txt
7+
LABEL x="really stage 1"
8+
9+
FROM stage0

‎dockerclient/testdata/multistage/stage0.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
really stage 0

‎dockerclient/testdata/multistage/stage1.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
really stage 1

0 commit comments

Comments
 (0)