Skip to content

Commit 0d999dd

Browse files
mislavmislav
committed
Rework local extensions for Windows
Replace the implementation that relied on symlinks with the one that create regular files that act like symlinks: they contain a reference to the local directory where to find the extension.
1 parent 4b499be commit 0d999dd

File tree

11 files changed

+100
-61
lines changed

11 files changed

+100
-61
lines changed

‎cmd/gh/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func mainRun() exitCode {
166166
}
167167
}
168168
}
169-
for _, ext := range cmdFactory.ExtensionManager.List() {
169+
for _, ext := range cmdFactory.ExtensionManager.List(false) {
170170
if strings.HasPrefix(ext.Name(), toComplete) {
171171
results = append(results, ext.Name())
172172
}

‎pkg/cmd/alias/set/set.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
8282
return true
8383
}
8484

85-
for _, ext := range f.ExtensionManager.List() {
85+
for _, ext := range f.ExtensionManager.List(false) {
8686
if ext.Name() == split[0] {
8787
return true
8888
}

‎pkg/cmd/alias/set/set_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func runCommand(cfg config.Config, isTTY bool, cli string, in string) (*test.Cmd
3030
return cfg, nil
3131
},
3232
ExtensionManager: &extensions.ExtensionManagerMock{
33-
ListFunc: func() []extensions.Extension {
33+
ListFunc: func(bool) []extensions.Extension {
3434
return []extensions.Extension{}
3535
},
3636
},

‎pkg/cmd/extensions/command.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func NewCmdExtensions(f *cmdutil.Factory) *cobra.Command {
3939
Short: "List installed extension commands",
4040
Args: cobra.NoArgs,
4141
RunE: func(cmd *cobra.Command, args []string) error {
42-
cmds := m.List()
42+
cmds := m.List(true)
4343
if len(cmds) == 0 {
4444
return errors.New("no extensions installed")
4545
}
@@ -158,7 +158,7 @@ func checkValidExtension(rootCmd *cobra.Command, m extensions.ExtensionManager,
158158
return fmt.Errorf("%q matches the name of a built-in command", commandName)
159159
}
160160

161-
for _, ext := range m.List() {
161+
for _, ext := range m.List(false) {
162162
if ext.Name() == commandName {
163163
return fmt.Errorf("there is already an installed extension that provides the %q command", commandName)
164164
}

‎pkg/cmd/extensions/command_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestNewCmdExtensions(t *testing.T) {
3535
name: "install an extension",
3636
args: []string{"install", "owner/gh-some-ext"},
3737
managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
38-
em.ListFunc = func() []extensions.Extension {
38+
em.ListFunc = func(bool) []extensions.Extension {
3939
return []extensions.Extension{}
4040
}
4141
em.InstallFunc = func(s string, out, errOut io.Writer) error {
@@ -54,7 +54,7 @@ func TestNewCmdExtensions(t *testing.T) {
5454
name: "install an extension with same name as existing extension",
5555
args: []string{"install", "owner/gh-existing-ext"},
5656
managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
57-
em.ListFunc = func() []extensions.Extension {
57+
em.ListFunc = func(bool) []extensions.Extension {
5858
e := &Extension{path: "owner2/gh-existing-ext"}
5959
return []extensions.Extension{e}
6060
}
@@ -150,7 +150,7 @@ func TestNewCmdExtensions(t *testing.T) {
150150
name: "list extensions",
151151
args: []string{"list"},
152152
managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
153-
em.ListFunc = func() []extensions.Extension {
153+
em.ListFunc = func(bool) []extensions.Extension {
154154
ex1 := &Extension{path: "cli/gh-test", url: "https://github.com/cli/gh-test", updateAvailable: false}
155155
ex2 := &Extension{path: "cli/gh-test2", url: "https://github.com/cli/gh-test2", updateAvailable: true}
156156
return []extensions.Extension{ex1, ex2}
@@ -215,7 +215,7 @@ func Test_checkValidExtension(t *testing.T) {
215215
rootCmd.AddCommand(&cobra.Command{Use: "auth"})
216216

217217
m := &extensions.ExtensionManagerMock{
218-
ListFunc: func() []extensions.Extension {
218+
ListFunc: func(bool) []extensions.Extension {
219219
return []extensions.Extension{
220220
&extensions.ExtensionMock{
221221
NameFunc: func() string { return "screensaver" },

‎pkg/cmd/extensions/extension.go‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
package extensions
22

33
import (
4-
"errors"
5-
"os"
64
"path/filepath"
75
"strings"
86
)
97

108
type Extension struct {
119
path string
1210
url string
11+
isLocal bool
1312
updateAvailable bool
1413
}
1514

@@ -26,20 +25,7 @@ func (e *Extension) URL() string {
2625
}
2726

2827
func (e *Extension) IsLocal() bool {
29-
dir := filepath.Dir(e.path)
30-
fileInfo, err := os.Lstat(dir)
31-
if err != nil {
32-
return false
33-
}
34-
// Check if extension is a symlink
35-
if fileInfo.Mode()&os.ModeSymlink != 0 {
36-
return true
37-
}
38-
// Check if extension does not have a git directory
39-
if _, err = os.Stat(filepath.Join(dir, ".git")); errors.Is(err, os.ErrNotExist) {
40-
return true
41-
}
42-
return false
28+
return e.isLocal
4329
}
4430

4531
func (e *Extension) UpdateAvailable() bool {

‎pkg/cmd/extensions/manager.go‎

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Wri
4444
extName := args[0]
4545
forwardArgs := args[1:]
4646

47-
for _, e := range m.list(false) {
47+
exts, _ := m.list(false)
48+
for _, e := range exts {
4849
if e.Name() == extName {
4950
exe = e.Path()
5051
break
@@ -77,34 +78,56 @@ func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Wri
7778
return true, externalCmd.Run()
7879
}
7980

80-
func (m *Manager) List() []extensions.Extension {
81-
return m.list(true)
81+
func (m *Manager) List(includeMetadata bool) []extensions.Extension {
82+
exts, _ := m.list(includeMetadata)
83+
return exts
8284
}
8385

84-
func (m *Manager) list(includeMetadata bool) []extensions.Extension {
86+
func (m *Manager) list(includeMetadata bool) ([]extensions.Extension, error) {
8587
dir := m.installDir()
8688
entries, err := ioutil.ReadDir(dir)
8789
if err != nil {
88-
return nil
90+
return nil, err
8991
}
92+
9093
var results []extensions.Extension
9194
for _, f := range entries {
92-
if !strings.HasPrefix(f.Name(), "gh-") || !(f.IsDir() || f.Mode()&os.ModeSymlink != 0) {
95+
if !strings.HasPrefix(f.Name(), "gh-") {
9396
continue
9497
}
9598
var remoteUrl string
96-
var updateAvailable bool
97-
if includeMetadata {
98-
remoteUrl = m.getRemoteUrl(f.Name())
99-
updateAvailable = m.checkUpdateAvailable(f.Name())
99+
updateAvailable := false
100+
isLocal := false
101+
exePath := filepath.Join(dir, f.Name(), f.Name())
102+
if f.IsDir() {
103+
if includeMetadata {
104+
remoteUrl = m.getRemoteUrl(f.Name())
105+
updateAvailable = m.checkUpdateAvailable(f.Name())
106+
}
107+
} else {
108+
isLocal = true
109+
if f.Mode()&os.ModeSymlink == 0 {
110+
// if this is a regular file, its contents is the local directory of the extension
111+
exeFile, err := os.Open(filepath.Join(dir, f.Name()))
112+
if err != nil {
113+
return nil, err
114+
}
115+
b := make([]byte, 1024)
116+
n, err := exeFile.Read(b)
117+
if err != nil {
118+
return nil, err
119+
}
120+
exePath = filepath.Join(strings.TrimSpace(string(b[:n])), f.Name())
121+
}
100122
}
101123
results = append(results, &Extension{
102-
path: filepath.Join(dir, f.Name(), f.Name()),
124+
path: exePath,
103125
url: remoteUrl,
126+
isLocal: isLocal,
104127
updateAvailable: updateAvailable,
105128
})
106129
}
107-
return results
130+
return results, nil
108131
}
109132

110133
func (m *Manager) getRemoteUrl(extension string) string {
@@ -146,8 +169,19 @@ func (m *Manager) checkUpdateAvailable(extension string) bool {
146169

147170
func (m *Manager) InstallLocal(dir string) error {
148171
name := filepath.Base(dir)
149-
targetDir := filepath.Join(m.installDir(), name)
150-
return os.Symlink(dir, targetDir)
172+
targetLink := filepath.Join(m.installDir(), name)
173+
if err := os.MkdirAll(filepath.Dir(targetLink), 0755); err != nil {
174+
return err
175+
}
176+
// Create a regular file that contains the location of the directory where to find this extension. We
177+
// avoid relying on symlinks because creating them on Windows requires administrator privileges.
178+
f, err := os.OpenFile(targetLink, os.O_WRONLY|os.O_CREATE, 0644)
179+
if err != nil {
180+
return err
181+
}
182+
defer f.Close()
183+
_, err = f.WriteString(dir)
184+
return err
151185
}
152186

153187
func (m *Manager) Install(cloneURL string, stdout, stderr io.Writer) error {
@@ -173,7 +207,7 @@ func (m *Manager) Upgrade(name string, force bool, stdout, stderr io.Writer) err
173207
return err
174208
}
175209

176-
exts := m.List()
210+
exts := m.List(false)
177211
if len(exts) == 0 {
178212
return errors.New("no extensions installed")
179213
}

‎pkg/cmd/extensions/manager_test.go‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestManager_List(t *testing.T) {
4848
assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-two", "gh-two")))
4949

5050
m := newTestManager(tempDir)
51-
exts := m.List()
51+
exts := m.List(false)
5252
assert.Equal(t, 2, len(exts))
5353
assert.Equal(t, "hello", exts[0].Name())
5454
assert.Equal(t, "two", exts[1].Name())
@@ -94,7 +94,7 @@ func TestManager_Upgrade_AllExtensions(t *testing.T) {
9494
tempDir := t.TempDir()
9595
assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello")))
9696
assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-two", "gh-two")))
97-
assert.NoError(t, stubLocalExtension(filepath.Join(tempDir, "extensions", "gh-local", "gh-local")))
97+
assert.NoError(t, stubLocalExtension(tempDir, filepath.Join(tempDir, "extensions", "gh-local", "gh-local")))
9898

9999
m := newTestManager(tempDir)
100100

@@ -139,7 +139,7 @@ func TestManager_Upgrade_RemoteExtension(t *testing.T) {
139139

140140
func TestManager_Upgrade_LocalExtension(t *testing.T) {
141141
tempDir := t.TempDir()
142-
assert.NoError(t, stubLocalExtension(filepath.Join(tempDir, "extensions", "gh-local", "gh-local")))
142+
assert.NoError(t, stubLocalExtension(tempDir, filepath.Join(tempDir, "extensions", "gh-local", "gh-local")))
143143

144144
m := newTestManager(tempDir)
145145

@@ -203,12 +203,7 @@ func TestManager_Install(t *testing.T) {
203203
}
204204

205205
func stubExtension(path string) error {
206-
dir := filepath.Dir(path)
207-
gitDir := filepath.Join(dir, ".git")
208-
if err := os.MkdirAll(dir, 0755); err != nil {
209-
return err
210-
}
211-
if err := os.Mkdir(gitDir, 0755); err != nil {
206+
if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
212207
return err
213208
}
214209
f, err := os.OpenFile(path, os.O_CREATE, 0755)
@@ -218,11 +213,28 @@ func stubExtension(path string) error {
218213
return f.Close()
219214
}
220215

221-
func stubLocalExtension(path string) error {
222-
if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
216+
func stubLocalExtension(tempDir, path string) error {
217+
extDir, err := os.MkdirTemp(tempDir, "local-ext")
218+
if err != nil {
223219
return err
224220
}
225-
f, err := os.OpenFile(path, os.O_CREATE, 0755)
221+
extFile, err := os.OpenFile(filepath.Join(extDir, filepath.Base(path)), os.O_CREATE, 0755)
222+
if err != nil {
223+
return err
224+
}
225+
if err := extFile.Close(); err != nil {
226+
return err
227+
}
228+
229+
linkPath := filepath.Dir(path)
230+
if err := os.MkdirAll(filepath.Dir(linkPath), 0755); err != nil {
231+
return err
232+
}
233+
f, err := os.OpenFile(linkPath, os.O_WRONLY|os.O_CREATE, 0644)
234+
if err != nil {
235+
return err
236+
}
237+
_, err = f.WriteString(extDir)
226238
if err != nil {
227239
return err
228240
}

‎pkg/cmd/root/help.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, args []string) {
145145
}
146146

147147
if isRootCmd(command) {
148-
if exts := f.ExtensionManager.List(); len(exts) > 0 {
148+
if exts := f.ExtensionManager.List(false); len(exts) > 0 {
149149
var names []string
150150
for _, ext := range exts {
151151
names = append(names, ext.Name())

‎pkg/extensions/extension.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import (
44
"io"
55
)
66

7-
//go:generate moq -out extension_mock.go . Extension
7+
//go:generate moq -rm -out extension_mock.go . Extension
88
type Extension interface {
99
Name() string
1010
Path() string
@@ -13,9 +13,9 @@ type Extension interface {
1313
UpdateAvailable() bool
1414
}
1515

16-
//go:generate moq -out manager_mock.go . ExtensionManager
16+
//go:generate moq -rm -out manager_mock.go . ExtensionManager
1717
type ExtensionManager interface {
18-
List() []Extension
18+
List(includeMetadata bool) []Extension
1919
Install(url string, stdout, stderr io.Writer) error
2020
InstallLocal(dir string) error
2121
Upgrade(name string, force bool, stdout, stderr io.Writer) error

0 commit comments

Comments
 (0)