Skip to content
This repository was archived by the owner on Jan 16, 2025. It is now read-only.

Commit 8d46bab

Browse files
jiangxinjiangxin
committed
After load manifests, check and fixup missing attrs
Check and fixup missing attributes after load manifests. + If some attributes (such as "name") are missing, report errors. + If "path" attributes are missing, fill with value of "name" attr. Reported-by: zoujunhua <zoujunhua86@gmail.com> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
1 parent 44c6a88 commit 8d46bab

File tree

2 files changed

+320
-4
lines changed

2 files changed

+320
-4
lines changed

‎manifest/manifest.go‎

Lines changed: 125 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package manifest
1717

1818
import (
1919
"encoding/xml"
20+
"errors"
2021
"fmt"
2122
"io/ioutil"
2223
"os"
@@ -149,6 +150,127 @@ type Include struct {
149150
Name string `xml:"name,attr,omitempty"`
150151
}
151152

153+
// CheckAndFixup will fixup "Manifest" element
154+
func (v *Manifest) CheckAndFixup() error {
155+
for i := range v.Remotes {
156+
if err := v.Remotes[i].CheckAndFixup(); err != nil {
157+
return err
158+
}
159+
}
160+
for i := range v.Projects {
161+
if err := v.Projects[i].CheckAndFixup(); err != nil {
162+
return err
163+
}
164+
}
165+
for i := range v.RemoveProjects {
166+
if err := v.RemoveProjects[i].CheckAndFixup(); err != nil {
167+
return err
168+
}
169+
}
170+
for i := range v.ExtendProjects {
171+
if err := v.ExtendProjects[i].CheckAndFixup(); err != nil {
172+
return err
173+
}
174+
}
175+
for i := range v.Includes {
176+
if err := v.Includes[i].CheckAndFixup(); err != nil {
177+
return err
178+
}
179+
}
180+
181+
return nil
182+
}
183+
184+
// CheckAndFixup will fixup "Remote" element
185+
func (v *Remote) CheckAndFixup() error {
186+
if v.Name == "" {
187+
return errors.New("\"remote\" element has empty \"name\"")
188+
}
189+
return nil
190+
}
191+
192+
// CheckAndFixup will fixup "Include" element
193+
func (v *Include) CheckAndFixup() error {
194+
if v.Name == "" {
195+
return errors.New("\"include\" element has empty \"name\"")
196+
}
197+
return nil
198+
}
199+
200+
// CheckAndFixup will fixup "Project" element
201+
func (v *Project) CheckAndFixup() error {
202+
if v.Name == "" {
203+
return errors.New("\"project\" element has empty \"name\"")
204+
}
205+
v.Name = cleanPath(v.Name)
206+
if v.Path == "" {
207+
v.Path = v.Name
208+
} else {
209+
v.Path = cleanPath(v.Path)
210+
}
211+
for i := range v.CopyFiles {
212+
if err := v.CopyFiles[i].CheckAndFixup(); err != nil {
213+
return err
214+
}
215+
}
216+
for i := range v.LinkFiles {
217+
if err := v.LinkFiles[i].CheckAndFixup(); err != nil {
218+
return err
219+
}
220+
}
221+
for i := range v.Projects {
222+
if err := v.Projects[i].CheckAndFixup(); err != nil {
223+
return err
224+
}
225+
}
226+
return nil
227+
}
228+
229+
// CheckAndFixup will fixup "RemoveProject" element
230+
func (v *RemoveProject) CheckAndFixup() error {
231+
if v.Name == "" {
232+
return errors.New("\"remove-project\" element has empty \"name\"")
233+
}
234+
v.Name = cleanPath(v.Name)
235+
return nil
236+
}
237+
238+
// CheckAndFixup will fixup "ExtendProject" element
239+
func (v *ExtendProject) CheckAndFixup() error {
240+
if v.Name == "" {
241+
return errors.New("\"extend-project\" element has empty \"name\"")
242+
}
243+
v.Name = cleanPath(v.Name)
244+
if v.Path == "" {
245+
v.Path = v.Name
246+
} else {
247+
v.Path = cleanPath(v.Path)
248+
}
249+
return nil
250+
}
251+
252+
// CheckAndFixup will fixup "copyfile" element
253+
func (v *CopyFile) CheckAndFixup() error {
254+
if v.Src == "" {
255+
return errors.New("\"copyfile\" element has empty \"src\"")
256+
}
257+
if v.Dest == "" {
258+
return errors.New("\"copyfile\" element has empty \"dest\"")
259+
}
260+
return nil
261+
}
262+
263+
// CheckAndFixup will fixup "linkfile" element
264+
func (v *LinkFile) CheckAndFixup() error {
265+
if v.Src == "" {
266+
return errors.New("\"linkfile\" element has empty \"src\"")
267+
}
268+
if v.Dest == "" {
269+
return errors.New("\"linkfile\" element has empty \"dest\"")
270+
}
271+
return nil
272+
}
273+
152274
// AllProjects returns all projects (include current project and all sub-projects)
153275
// of a project recursively.
154276
func (v *Project) AllProjects(parent *Project) []Project {
@@ -362,8 +484,6 @@ func (v *Manifest) Merge(m *Manifest) error {
362484
realPath[p.Path] = true
363485
}
364486
for _, p := range m.allProjects() {
365-
p.Name = cleanPath(p.Name)
366-
p.Path = cleanPath(p.Path)
367487
if realPath[p.Path] {
368488
return fmt.Errorf("duplicate path for project '%s' in '%s'",
369489
p.Path,
@@ -375,7 +495,6 @@ func (v *Manifest) Merge(m *Manifest) error {
375495

376496
rmPath := make(map[string]bool)
377497
for _, r := range m.RemoveProjects {
378-
r.Name = cleanPath(r.Name)
379498
rmPath[r.Name] = true
380499
}
381500
ps := []Project{}
@@ -390,7 +509,6 @@ func (v *Manifest) Merge(m *Manifest) error {
390509

391510
extPath := make(map[string]ExtendProject)
392511
for _, p := range m.ExtendProjects {
393-
p.Name = cleanPath(p.Name)
394512
extPath[p.Name] = p
395513
}
396514
for i, p := range v.Projects {
@@ -491,6 +609,9 @@ func parseXML(file string, depth int) ([]*Manifest, error) {
491609
if m == nil {
492610
return ms, nil
493611
}
612+
if err := m.CheckAndFixup(); err != nil {
613+
return ms, err
614+
}
494615
m.SourceFile = file
495616
ms = append(ms, m)
496617

‎test/t0226-sync-with-special-manifests-file.sh‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
#!/bin/sh
2+
3+
test_description="sync with special manifests file"
4+
5+
. ./lib/sharness.sh
6+
7+
# Create manifest repositories
8+
manifest_url="file://${HOME}/repositories/hello/manifests"
9+
10+
test_expect_success "setup" '
11+
(
12+
# create .repo file as a barrier, not find .repo deeper
13+
touch .repo &&
14+
mkdir repositories &&
15+
cd repositories &&
16+
ln -s "${REPO_TEST_REPOSITORIES}/drivers" . &&
17+
ln -s "${REPO_TEST_REPOSITORIES}/others" . &&
18+
mkdir hello &&
19+
cd hello &&
20+
ln -s "${REPO_TEST_REPOSITORIES}/hello/main.git" . &&
21+
ln -s "${REPO_TEST_REPOSITORIES}/hello/project1.git" . &&
22+
ln -s "${REPO_TEST_REPOSITORIES}/hello/project2.git" . &&
23+
ln -s "${REPO_TEST_REPOSITORIES}/hello/project1" . &&
24+
git clone --mirror \
25+
"${REPO_TEST_REPOSITORIES}/hello/manifests.git" \
26+
manifests.git
27+
)
28+
'
29+
30+
test_expect_success "setup manifests: no name attr in project element" '
31+
(
32+
git clone "${manifest_url}.git" manifests &&
33+
cd manifests &&
34+
git checkout master &&
35+
36+
cat >default.xml <<-EOF &&
37+
<?xml version="1.0" encoding="UTF-8"?>
38+
<manifest>
39+
<remote name="aone"
40+
alias="origin"
41+
fetch="."
42+
review="https://example.com" />
43+
<remote name="driver"
44+
fetch=".."
45+
revision="Maint"
46+
review="https://example.com" />
47+
<default remote="aone"
48+
revision="master"
49+
sync-j="4" />
50+
<project name="main" groups="app">
51+
<copyfile src="VERSION" dest="VERSION"></copyfile>
52+
<linkfile src="Makefile" dest="Makefile"></linkfile>
53+
</project>
54+
<project name="project1" path="projects/app1" groups="app">
55+
<project name="module1" groups="app" revision="refs/tags/v1.0.0" />
56+
</project>
57+
<project path="projects/app2" groups="app"/>
58+
<project name="drivers/driver1" path="drivers/driver-1" groups="drivers" remote="driver" />
59+
<project name="drivers/driver2" path="drivers/driver-2" groups="notdefault,drivers" remote="driver" />
60+
</manifest>
61+
<!-- no name attr in project element -->
62+
EOF
63+
git add -u &&
64+
git commit -m "default.xml: no name attr in project element" &&
65+
git push
66+
)
67+
'
68+
69+
test_expect_success "fail to sync: empty name attr in project" '
70+
(
71+
mkdir work1 &&
72+
cd work1 &&
73+
git-repo init -u "$manifest_url" &&
74+
test_must_fail git-repo sync \
75+
--mock-ssh-info-status 200 \
76+
--mock-ssh-info-response \
77+
"{\"host\":\"ssh.example.com\", \"port\":22, \"type\":\"agit\"}" >out 2>&1 &&
78+
grep "^FATAL" out >actual &&
79+
cat >expect <<-\EOF &&
80+
FATAL: "project" element has empty "name"
81+
EOF
82+
test_cmp expect actual
83+
)
84+
'
85+
86+
87+
test_expect_success "setup manifests: no name attr in remote element" '
88+
(
89+
cd manifests &&
90+
91+
cat >default.xml <<-EOF &&
92+
<?xml version="1.0" encoding="UTF-8"?>
93+
<manifest>
94+
<remote
95+
alias="origin"
96+
fetch="."
97+
review="https://example.com" />
98+
<remote
99+
fetch=".."
100+
revision="Maint"
101+
review="https://example.com" />
102+
<default remote="aone"
103+
revision="master"
104+
sync-j="4" />
105+
<project name="main" groups="app">
106+
<copyfile src="VERSION" dest="VERSION"></copyfile>
107+
<linkfile src="Makefile" dest="Makefile"></linkfile>
108+
</project>
109+
<project name="project1" path="projects/app1" groups="app">
110+
<project name="module1" groups="app" revision="refs/tags/v1.0.0" />
111+
</project>
112+
<project name="projects/app2" path="projects/app2" groups="app"/>
113+
<project name="drivers/driver1" path="drivers/driver-1" groups="drivers" remote="driver" />
114+
<project name="drivers/driver2" path="drivers/driver-2" groups="notdefault,drivers" remote="driver" />
115+
</manifest>
116+
<!-- no name attr in remote element -->
117+
EOF
118+
git add -u &&
119+
git commit -m "default.xml: no name attr in remote element" &&
120+
git push
121+
)
122+
'
123+
124+
test_expect_success "fail to sync: empty name attr in remote element" '
125+
(
126+
mkdir work2 &&
127+
cd work2 &&
128+
git-repo init -u "$manifest_url" &&
129+
test_must_fail git-repo sync \
130+
--mock-ssh-info-status 200 \
131+
--mock-ssh-info-response \
132+
"{\"host\":\"ssh.example.com\", \"port\":22, \"type\":\"agit\"}" >out 2>&1 &&
133+
grep "^FATAL" out >actual &&
134+
cat >expect <<-\EOF &&
135+
FATAL: "remote" element has empty "name"
136+
EOF
137+
test_cmp expect actual
138+
)
139+
'
140+
141+
test_expect_success "setup manifests: no path attr in project element" '
142+
(
143+
cd manifests &&
144+
145+
cat >default.xml <<-EOF &&
146+
<?xml version="1.0" encoding="UTF-8"?>
147+
<manifest>
148+
<remote name="aone"
149+
alias="origin"
150+
fetch="."
151+
review="https://example.com" />
152+
<remote name="driver"
153+
fetch=".."
154+
revision="Maint"
155+
review="https://example.com" />
156+
<default remote="aone"
157+
revision="master"
158+
sync-j="4" />
159+
<project name="main" groups="app">
160+
<copyfile src="VERSION" dest="VERSION"></copyfile>
161+
<linkfile src="Makefile" dest="Makefile"></linkfile>
162+
</project>
163+
<project name="project1" path="projects/app1" groups="app">
164+
<project name="module1" groups="app" revision="refs/tags/v1.0.0" />
165+
</project>
166+
<project name="project2" path="projects/app2" groups="app"/>
167+
<project name="drivers/driver1" groups="drivers" remote="driver" />
168+
<project name="drivers/driver2" groups="notdefault,drivers" remote="driver" />
169+
</manifest>
170+
<!-- no path attr in project element -->
171+
EOF
172+
git add -u &&
173+
git commit -m "default.xml: no path attributes for some projects" &&
174+
git push
175+
)
176+
'
177+
178+
test_expect_success "sync ok for project without path attr" '
179+
(
180+
mkdir work3 &&
181+
cd work3 &&
182+
git-repo init -u "$manifest_url" &&
183+
git-repo sync \
184+
--mock-ssh-info-status 200 \
185+
--mock-ssh-info-response \
186+
"{\"host\":\"ssh.example.com\", \"port\":22, \"type\":\"agit\"}" &&
187+
test -f main/VERSION &&
188+
test -f projects/app1/VERSION &&
189+
test -f projects/app2/VERSION &&
190+
test -f projects/app1/module1/VERSION &&
191+
test -f drivers/driver1/VERSION
192+
)
193+
'
194+
195+
test_done

0 commit comments

Comments
 (0)