Skip to content

Commit aab8656

Browse files
mvo5mvo5
authored
Merge pull request canonical#11038 from valentindavid/valentindavid/completion-writable-dir
many: install bash completion files in writable directory
2 parents fe43ced + a1cb7ae commit aab8656

File tree

17 files changed

+519
-57
lines changed

17 files changed

+519
-57
lines changed

Diff for: ‎data/completion/bash/complete.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,10 @@ _complete_from_snap() {
102102
}
103103

104104
# this file can be sourced directly as e.g. /usr/lib/snapd/complete.sh, or via
105-
# a symlink from /usr/share/bash-completion/completions/. In the first case we
105+
# a symlink from /var/lib/snapd/desktop/bash-completion/completions/. In the first case we
106106
# want to load the default loader; in the second, the specific one.
107107
#
108-
if [[ "${BASH_SOURCE[0]}" =~ ^/usr/share/bash-completion/completions/ ]]; then
108+
if [[ "${BASH_SOURCE[0]}" =~ ^(/var/lib/snapd/desktop|/usr/share)/bash-completion/completions/ ]]; then
109109
complete -F _complete_from_snap "$1"
110110
else
111111

Diff for: ‎dirs/dirs.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ var (
125125
XdgRuntimeDirGlob string
126126

127127
CompletionHelperInCore string
128+
BashCompletionScript string
129+
LegacyCompletersDir string
128130
CompletersDir string
129131

130132
SystemFontsDir string
@@ -475,7 +477,9 @@ func SetRootDir(rootdir string) {
475477
XdgRuntimeDirGlob = filepath.Join(XdgRuntimeDirBase, "*/")
476478

477479
CompletionHelperInCore = filepath.Join(CoreLibExecDir, "etelpmoc.sh")
478-
CompletersDir = filepath.Join(rootdir, "/usr/share/bash-completion/completions/")
480+
BashCompletionScript = filepath.Join(rootdir, "/usr/share/bash-completion/bash_completion")
481+
LegacyCompletersDir = filepath.Join(rootdir, "/usr/share/bash-completion/completions/")
482+
CompletersDir = filepath.Join(rootdir, snappyDir, "desktop/bash-completion/completions/")
479483

480484
// These paths agree across all supported distros
481485
SystemFontsDir = filepath.Join(rootdir, "/usr/share/fonts")

Diff for: ‎image/preseed/preseed_classic_test.go

+2
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,8 @@ func (s *preseedSuite) TestReset(c *C) {
301301
// bash-completion symlinks
302302
{filepath.Join(dirs.CompletersDir, "foo.bar"), "/a/snapd/complete.sh"},
303303
{filepath.Join(dirs.CompletersDir, "foo"), "foo.bar"},
304+
{filepath.Join(dirs.LegacyCompletersDir, "foo.bar"), "/a/snapd/complete.sh"},
305+
{filepath.Join(dirs.LegacyCompletersDir, "foo"), "foo.bar"},
304306
}
305307

306308
for _, art := range artifacts {

Diff for: ‎image/preseed/reset.go

+47-37
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,50 @@ import (
3131
apparmor_sandbox "github.com/snapcore/snapd/sandbox/apparmor"
3232
)
3333

34+
// bash-completion symlinks; note there are symlinks that point at
35+
// completer, and symlinks that point at the completer symlinks.
36+
// e.g.
37+
// lxd.lxc -> /snap/core/current/usr/lib/snapd/complete.sh
38+
// lxc -> lxd.lxc
39+
func resetCompletionSymlinks(completersPath string) error {
40+
files, err := ioutil.ReadDir(completersPath)
41+
if err != nil && !os.IsNotExist(err) {
42+
return fmt.Errorf("error reading %s: %v", completersPath, err)
43+
}
44+
completeShSymlinks := make(map[string]string)
45+
var otherSymlinks []string
46+
47+
// pass 1: find all symlinks pointing at complete.sh
48+
for _, fileInfo := range files {
49+
if fileInfo.Mode()&os.ModeSymlink == 0 {
50+
continue
51+
}
52+
fullPath := filepath.Join(completersPath, fileInfo.Name())
53+
if dirs.IsCompleteShSymlink(fullPath) {
54+
if err := os.Remove(fullPath); err != nil {
55+
return fmt.Errorf("error removing symlink %s: %v", fullPath, err)
56+
}
57+
completeShSymlinks[fileInfo.Name()] = fullPath
58+
} else {
59+
otherSymlinks = append(otherSymlinks, fullPath)
60+
}
61+
}
62+
// pass 2: find all symlinks that point at the symlinks found in pass 1.
63+
for _, other := range otherSymlinks {
64+
target, err := os.Readlink(other)
65+
if err != nil {
66+
return fmt.Errorf("error reading symlink target of %s: %v", other, err)
67+
}
68+
if _, ok := completeShSymlinks[target]; ok {
69+
if err := os.Remove(other); err != nil {
70+
return fmt.Errorf("error removing symlink %s: %v", other, err)
71+
}
72+
}
73+
}
74+
75+
return nil
76+
}
77+
3478
// ResetPreseededChroot removes all preseeding artifacts from preseedChroot
3579
// (classic Ubuntu only).
3680
func ResetPreseededChroot(preseedChroot string) error {
@@ -130,43 +174,9 @@ func ResetPreseededChroot(preseedChroot string) error {
130174
}
131175
}
132176

133-
// bash-completion symlinks; note there are symlinks that point at
134-
// completer, and symlinks that point at the completer symlinks.
135-
// e.g.
136-
// lxd.lxc -> /snap/core/current/usr/lib/snapd/complete.sh
137-
// lxc -> lxd.lxc
138-
files, err := ioutil.ReadDir(filepath.Join(preseedChroot, dirs.CompletersDir))
139-
if err != nil && !os.IsNotExist(err) {
140-
return fmt.Errorf("error reading %s: %v", dirs.CompletersDir, err)
141-
}
142-
completeShSymlinks := make(map[string]string)
143-
var otherSymlinks []string
144-
145-
// pass 1: find all symlinks pointing at complete.sh
146-
for _, fileInfo := range files {
147-
if fileInfo.Mode()&os.ModeSymlink == 0 {
148-
continue
149-
}
150-
fullPath := filepath.Join(preseedChroot, dirs.CompletersDir, fileInfo.Name())
151-
if dirs.IsCompleteShSymlink(fullPath) {
152-
if err := os.Remove(fullPath); err != nil {
153-
return fmt.Errorf("error removing symlink %s: %v", fullPath, err)
154-
}
155-
completeShSymlinks[fileInfo.Name()] = fullPath
156-
} else {
157-
otherSymlinks = append(otherSymlinks, fullPath)
158-
}
159-
}
160-
// pass 2: find all symlinks that point at the symlinks found in pass 1.
161-
for _, other := range otherSymlinks {
162-
target, err := os.Readlink(other)
163-
if err != nil {
164-
return fmt.Errorf("error reading symlink target of %s: %v", other, err)
165-
}
166-
if _, ok := completeShSymlinks[target]; ok {
167-
if err := os.Remove(other); err != nil {
168-
return fmt.Errorf("error removing symlink %s: %v", other, err)
169-
}
177+
for _, completersPath := range []string{dirs.CompletersDir, dirs.LegacyCompletersDir} {
178+
if err := resetCompletionSymlinks(filepath.Join(preseedChroot, completersPath)); err != nil {
179+
return err
170180
}
171181
}
172182

Diff for: ‎overlord/snapstate/backend/aliases.go

+18-5
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,13 @@ func (b Backend) UpdateAliases(add []*Alias, remove []*Alias) error {
4646
return fmt.Errorf("cannot remove alias symlink: %v", err)
4747
}
4848
removed[alias.Name] = true
49-
completer := filepath.Join(dirs.CompletersDir, alias.Name)
50-
target, err := os.Readlink(completer)
51-
if err != nil || target != alias.Target {
52-
continue
49+
for _, completersPath := range []string{dirs.CompletersDir, dirs.LegacyCompletersDir} {
50+
completer := filepath.Join(completersPath, alias.Name)
51+
target, err := os.Readlink(completer)
52+
if err == nil && target == alias.Target {
53+
os.Remove(completer)
54+
}
5355
}
54-
os.Remove(completer)
5556
}
5657

5758
for _, alias := range add {
@@ -68,8 +69,20 @@ func (b Backend) UpdateAliases(add []*Alias, remove []*Alias) error {
6869
return fmt.Errorf("cannot create alias symlink: %v", err)
6970
}
7071

72+
removeLegacy := true
7173
if dirs.IsCompleteShSymlink(filepath.Join(dirs.CompletersDir, alias.Target)) {
7274
os.Symlink(alias.Target, filepath.Join(dirs.CompletersDir, alias.Name))
75+
} else if dirs.IsCompleteShSymlink(filepath.Join(dirs.LegacyCompletersDir, alias.Target)) {
76+
os.Symlink(alias.Target, filepath.Join(dirs.LegacyCompletersDir, alias.Name))
77+
removeLegacy = false
78+
}
79+
80+
if removeLegacy {
81+
completer := filepath.Join(dirs.LegacyCompletersDir, alias.Name)
82+
target, err := os.Readlink(completer)
83+
if err == nil && target == alias.Target {
84+
os.Remove(completer)
85+
}
7386
}
7487
}
7588
return nil

Diff for: ‎overlord/snapstate/backend/aliases_test.go

+65-2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
. "gopkg.in/check.v1"
2828

2929
"github.com/snapcore/snapd/dirs"
30+
"github.com/snapcore/snapd/osutil"
3031
"github.com/snapcore/snapd/overlord/snapstate/backend"
3132
)
3233

@@ -52,6 +53,7 @@ func (s *aliasesSuite) SetUpTest(c *C) {
5253
err := os.MkdirAll(dirs.SnapBinariesDir, 0755)
5354
c.Assert(err, IsNil)
5455
c.Assert(os.MkdirAll(dirs.CompletersDir, 0755), IsNil)
56+
c.Assert(os.MkdirAll(dirs.LegacyCompletersDir, 0755), IsNil)
5557
c.Assert(os.MkdirAll(filepath.Dir(dirs.CompleteShPath(s.base)), 0755), IsNil)
5658
c.Assert(ioutil.WriteFile(dirs.CompleteShPath(s.base), nil, 0644), IsNil)
5759
}
@@ -92,7 +94,7 @@ func (s *aliasesSuite) TestMissingAliases(c *C) {
9294
c.Check(missingCs, DeepEquals, []*backend.Alias{{Name: "foo", Target: "x.foo"}})
9395
}
9496

95-
func matchingAliases(aliases []*backend.Alias) (matchingAs, matchingCs []*backend.Alias, err error) {
97+
func matchingAliasesCommon(aliases []*backend.Alias, completersDir string) (matchingAs, matchingCs []*backend.Alias, err error) {
9698
for _, cand := range aliases {
9799
target, err := os.Readlink(filepath.Join(dirs.SnapBinariesDir, cand.Name))
98100
if err == nil {
@@ -103,7 +105,7 @@ func matchingAliases(aliases []*backend.Alias) (matchingAs, matchingCs []*backen
103105
return nil, nil, err
104106
}
105107

106-
target, err = os.Readlink(filepath.Join(dirs.CompletersDir, cand.Name))
108+
target, err = os.Readlink(filepath.Join(completersDir, cand.Name))
107109
if err == nil {
108110
if target == cand.Target {
109111
matchingCs = append(matchingCs, cand)
@@ -115,6 +117,14 @@ func matchingAliases(aliases []*backend.Alias) (matchingAs, matchingCs []*backen
115117
return matchingAs, matchingCs, nil
116118
}
117119

120+
func matchingAliases(aliases []*backend.Alias) (matchingAs, matchingCs []*backend.Alias, err error) {
121+
return matchingAliasesCommon(aliases, dirs.CompletersDir)
122+
}
123+
124+
func matchingAliasesLegacy(aliases []*backend.Alias) (matchingAs, matchingCs []*backend.Alias, err error) {
125+
return matchingAliasesCommon(aliases, dirs.LegacyCompletersDir)
126+
}
127+
118128
func (s *aliasesSuite) TestMatchingAliases(c *C) {
119129
err := os.Symlink("x.foo", filepath.Join(dirs.SnapBinariesDir, "foo"))
120130
c.Assert(err, IsNil)
@@ -147,6 +157,12 @@ func mkCompleters(c *C, base string, apps ...string) {
147157
}
148158
}
149159

160+
func mkCompletersLegacy(c *C, base string, apps ...string) {
161+
for _, app := range apps {
162+
c.Assert(os.Symlink(dirs.CompleteShPath(base), filepath.Join(dirs.LegacyCompletersDir, app)), IsNil)
163+
}
164+
}
165+
150166
func (s *aliasesSuite) TestUpdateAliasesAddWithCompleter(c *C) {
151167
mkCompleters(c, s.base, "x.bar", "x.foo")
152168
aliases := []*backend.Alias{{Name: "foo", Target: "x.foo"}, {Name: "bar", Target: "x.bar"}}
@@ -160,6 +176,38 @@ func (s *aliasesSuite) TestUpdateAliasesAddWithCompleter(c *C) {
160176
c.Check(matchingCs, DeepEquals, aliases)
161177
}
162178

179+
func (s *aliasesSuite) TestUpdateAliasesAddWithLegacyCompleter(c *C) {
180+
mkCompletersLegacy(c, s.base, "x.bar", "x.foo")
181+
aliases := []*backend.Alias{{Name: "foo", Target: "x.foo"}, {Name: "bar", Target: "x.bar"}}
182+
183+
err := s.be.UpdateAliases(aliases, nil)
184+
c.Assert(err, IsNil)
185+
186+
matchingAs, matchingCs, err := matchingAliasesLegacy(aliases)
187+
c.Assert(err, IsNil)
188+
c.Check(matchingAs, DeepEquals, aliases)
189+
c.Check(matchingCs, DeepEquals, aliases)
190+
}
191+
192+
func (s *aliasesSuite) TestUpdateAliasesAddWithLegacyCompleterMigrate(c *C) {
193+
mkCompleters(c, s.base, "x.bar", "x.foo")
194+
aliases := []*backend.Alias{{Name: "foo", Target: "x.foo"}, {Name: "bar", Target: "x.bar"}}
195+
196+
c.Assert(os.Symlink("foo", filepath.Join(dirs.LegacyCompletersDir, "x.foo")), IsNil)
197+
c.Assert(os.Symlink("bar", filepath.Join(dirs.LegacyCompletersDir, "x.bar")), IsNil)
198+
199+
err := s.be.UpdateAliases(aliases, nil)
200+
c.Assert(err, IsNil)
201+
202+
matchingAs, matchingCs, err := matchingAliases(aliases)
203+
c.Assert(err, IsNil)
204+
c.Check(matchingAs, DeepEquals, aliases)
205+
c.Check(matchingCs, DeepEquals, aliases)
206+
207+
c.Check(osutil.FileExists(filepath.Join(dirs.LegacyCompletersDir, "x.foo")), Equals, false)
208+
c.Check(osutil.FileExists(filepath.Join(dirs.LegacyCompletersDir, "x.bar")), Equals, false)
209+
}
210+
163211
func (s *aliasesSuite) TestUpdateAliasesAddIdempot(c *C) {
164212
aliases := []*backend.Alias{{Name: "foo", Target: "x.foo"}, {Name: "bar", Target: "x.bar"}}
165213

@@ -242,6 +290,21 @@ func (s *aliasesSuite) TestUpdateAliasesWithCompleterRemove(c *C) {
242290
c.Check(matchingCs, HasLen, 0)
243291
}
244292

293+
func (s *aliasesSuite) TestUpdateAliasesWithLegacyCompleterRemove(c *C) {
294+
aliases := []*backend.Alias{{Name: "foo", Target: "x.foo"}, {Name: "bar", Target: "x.bar"}}
295+
err := s.be.UpdateAliases(aliases, nil)
296+
c.Assert(err, IsNil)
297+
298+
c.Assert(os.Symlink("foo", filepath.Join(dirs.LegacyCompletersDir, "x.foo")), IsNil)
299+
c.Assert(os.Symlink("bar", filepath.Join(dirs.LegacyCompletersDir, "x.bar")), IsNil)
300+
301+
err = s.be.UpdateAliases(nil, aliases)
302+
c.Assert(err, IsNil)
303+
304+
c.Check(osutil.FileExists(filepath.Join(dirs.LegacyCompletersDir, "x.foo")), Equals, false)
305+
c.Check(osutil.FileExists(filepath.Join(dirs.LegacyCompletersDir, "x.bar")), Equals, false)
306+
}
307+
245308
func (s *aliasesSuite) TestUpdateAliasesRemoveIdempot(c *C) {
246309
aliases := []*backend.Alias{{Name: "foo", Target: "x.foo"}, {Name: "bar", Target: "x.bar"}}
247310

Diff for: ‎snap/info.go

+5
Original file line numberDiff line numberDiff line change
@@ -1061,6 +1061,11 @@ func (app *AppInfo) CompleterPath() string {
10611061
return filepath.Join(dirs.CompletersDir, JoinSnapApp(app.Snap.InstanceName(), app.Name))
10621062
}
10631063

1064+
// CompleterPath returns the legacy path to the completer snippet for the app binary.
1065+
func (app *AppInfo) LegacyCompleterPath() string {
1066+
return filepath.Join(dirs.LegacyCompletersDir, JoinSnapApp(app.Snap.InstanceName(), app.Name))
1067+
}
1068+
10641069
func (app *AppInfo) launcherCommand(command string) string {
10651070
if command != "" {
10661071
command = " " + command

Diff for: ‎tests/completion/indirect/task.exp

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ send "source /usr/share/bash-completion/bash_completion\n"
55
next
66
if {$::env(_COMPLETE_SH)} {
77
# because this sources complete.sh, it won't be using the snippets
8-
send "source $::env(SPREAD_PATH)/data/completion/complete.sh\n"
8+
send "source $::env(SPREAD_PATH)/data/completion/bash/complete.sh\n"
99
next
1010
}
1111
chat "$::env(CMD) \t\t" $::env(_OUT0)

Diff for: ‎tests/completion/indirect/task.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ execute: |
2222
#shellcheck disable=SC1090
2323
source "${SPREAD_PATH}/${SPREAD_SUITE}/${SPREAD_VARIANT}.vars"
2424
export _OUT0 _OUT1 _OUT2 _KEY1 _KEY2 _COMP
25+
export XDG_DATA_DIRS="${XDG_DATA_DIRS:-}${XDG_DATA_DIRS:+:}/var/lib/snapd/desktop"
2526
for c in test-snapd-complexion test-snapd-complexion.two cplx cplx2; do
2627
sudo CMD=$c "PATH=$PATH" _COMPLETE_SH=true -E -u test expect -d -f "$d"/task.exp
2728
sudo CMD=$c "PATH=$PATH" _COMPLETE_SH=false -E -u test expect -d -f "$d"/task.exp

Diff for: ‎tests/completion/lib.exp0

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,5 @@ proc brexit {} {
6666
# tests to fail.
6767
spawn env -u PS1 bash --norc -i
6868
next
69-
send ". /usr/share/bash-completion/bash_completion; . ../../../data/completion/snap\n"
69+
send ". /usr/share/bash-completion/bash_completion; . ../../../data/completion/bash/snap\n"
7070
next

Diff for: ‎tests/completion/snippets/task.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@ execute: |
2020
#shellcheck disable=SC1090
2121
source "${SPREAD_PATH}/${SPREAD_SUITE}/${SPREAD_VARIANT}.vars"
2222
export _OUT0 _OUT1 _OUT2 _KEY1 _KEY2 _COMP
23+
export XDG_DATA_DIRS="${XDG_DATA_DIRS:-}${XDG_DATA_DIRS:+:}/var/lib/snapd/desktop"
2324
sudo PATH="$PATH" -E -u test expect -d -f "$d"/task.exp

Diff for: ‎tests/core/bash-completion/task.yaml

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
summary: bash completion
2+
3+
systems:
4+
- ubuntu-core-22-*
5+
6+
prepare: |
7+
snap install strace-static --edge
8+
cp ../../lib/snaps/test-snapd-complexion/meta/snap.yaml snap.yaml.bak
9+
cat <<EOF >>../../lib/snaps/test-snapd-complexion/meta/snap.yaml
10+
base: core22
11+
EOF
12+
cd ../../lib/snaps/test-snapd-complexion || exit 1
13+
snap try --devmode
14+
snap alias test-snapd-complexion cplx
15+
snap alias test-snapd-complexion.two cplx2
16+
17+
restore: |
18+
snap remove --purge test-snapd-complexion
19+
mv snap.yaml.bak ../../lib/snaps/test-snapd-complexion/meta/snap.yaml
20+
21+
execute: |
22+
for c in test-snapd-complexion test-snapd-complexion.two cplx cplx2; do
23+
python3 test-completion.py "${PWD}/test-rc" "${c}"
24+
done

0 commit comments

Comments
 (0)