feat: allow global host commands to be run from anywhere, for #4416 #5702

GuySartorelli · 2024-01-17T09:52:48Z

The Issue

Custom global host commands currently only be run inside a project directory - even if they don't need a project in order to do their stuff.

A great example of this is ddev self-upgrade which should be able to run from anywhere - but currently it has to be run from inside a project directory.

How This PR Solves The Issue

This PR adds a new CanRunGlobally annotation. If custom global host commands set this annotation to true, they will be added to the cobra command set, and can be run from anywhere, even outside project directories.

Manual Testing Instructions

Run ddev help - you should see the self-upgrade command there.
Run ddev self-upgrade - it should work from anywhere, even outside a project directory.
Add ## CanRunGlobally: true to a custom global host command, then perform the first two steps again, but with your custom command rather than self-upgrade.

Automated Testing Overview

I've added checks for the following the following:

Check a global host command with CanRunGlobally: true shows and can be run from arbitrary working directories
Check a global host command with CanRunGlobally: true still shows and can be run from inside a project directory
Check commands without CanRunGlobally: true do not show from arbitrary working directories

This is my first time really looking at how to add tests and I've gone in with a fairly light touch - if you want more test coverage I'll be happy to add it.

Release/Deployment Notes

Anyone who maintains a DDEV extension or who has some custom commands in projects they manage should review their custom global host commands, and add the new CanRunGlobally annotation to any commands that should be allowed to run even outside of project directories.

github-actions · 2024-01-17T09:56:15Z

Download the artifacts for this pull request:

See Testing a PR

GuySartorelli · 2024-01-17T10:03:49Z

cmd/ddev/cmd/commands.go

+	if runtime.GOOS == "windows" {
+		windowsBashPath := util.FindBashPath()
+		if windowsBashPath == "" {
+			fmt.Println("Unable to find bash.exe in PATH, not loading custom commands")
+			return nil
+		}
+	}


This was further down, inside a loop for some reason - but it just needs to be run once and should be run early to avoid unnecessary processing

GuySartorelli · 2024-01-17T10:06:18Z

cmd/ddev/cmd/commands.go

+// addCustomCommandsFromDir adds the custom commands from inside a given directory
+func addCustomCommandsFromDir(rootCmd *cobra.Command, app *ddevapp.DdevApp, serviceDirOnHost string, commandFiles []string, isGlobalSet bool, commandsAdded map[string]int) error {


Most of the code in this function used to live directly in addCustomCommands() - but we need to be able to run it with different input depending on whether we're in a project directory or not and the easiest way to handle that seems to be refactoring this into its own function.

GuySartorelli · 2024-01-17T10:07:27Z

cmd/ddev/cmd/commands.go

+		} else {
+			_ = os.Setenv("DDEV_PROJECT_STATUS", "")
+		}


Not sure if we need this or not? I figured it's safer to have it and not need it, rather than not have it and find out later we do need it.

GuySartorelli · 2024-01-17T10:09:34Z

Docs CI failure seems to be for an unrelated broken link

stasadev · 2024-01-17T12:11:14Z

Wow!

The rules for PR titles say that I must use fixes #4416 in the PR title because an issue exists

You can use for #4416 instead of fixes #4416 .

Docs CI failure seems to be for an unrelated broken link

Restarted failed jobs.

GuySartorelli · 2024-01-20T07:16:12Z

I've added some tests and updated the "Automated Testing Overview" section of the PR description.
Let me know if you want anything changed, more test coverage, etc.

GuySartorelli · 2024-01-20T07:40:14Z

I tried to test this on gitpod using https://gitpod.io/#https://github.com/ddev/ddev/pulls/5702 but when I ran ddev -v it showed that it was actually running on commit 9bd9054 (i.e. the latest commit on the master branch)... so I'm not sure if I was doing something wrong there, or if something is misconfigured, or something?

Anyway, that was more trying that out for fun - I did run this locally and it worked as expected.

rfay · 2024-01-20T12:25:31Z

I just opened on gitpod and got

$ ddev --version
ddev version feaedb50

which is correct. I used the "open" button provided by the gitpod app,

It links to https://gitpod.io/?autostart=true#https://github.com/ddev/ddev/pull/5702 which should be the same as your result.

Would be interested to know what might have gone wrong for you. Gitpod is a great resource for these things.

I am interested to note that gitpod no longer adds a comment with the link. That was a nice feature.

I tried it with your link, https://gitpod.io/#https://github.com/ddev/ddev/pulls/5702 and sure enough, the code in /workspace/ddev was upstream/master instead of the PR.

Mine https://gitpod.io/?autostart=true#https://github.com/ddev/ddev/pull/5702 checks out the branch properly:

gitpod /workspace/ddev (20240116_GuySartorelli_truly-global-commands) $ git status
On branch 20240116_GuySartorelli_truly-global-commands
Your branch is up to date with 'origin/20240116_GuySartorelli_truly-global-commands'.

Yours, https://gitpod.io/#https://github.com/ddev/ddev/pulls/5702,

gitpod /workspace/ddev (master) $ git status
On branch master
Your branch is up to date with 'origin/master'.

I have to assume this is a new bug in gitpod? Or perhaps it's just

Wrong branch checked out (main) when opening a Pull Request gitpod-io/gitpod#13991

which never actually got fixed?

rfay · 2024-01-20T12:36:26Z

Ah, the problem is your URL had pulls in it, not pull. (And that's because the doc says pulls). (It works fine with pull). I'll do docs PR.

Per #5702 (comment) we had a typo in the URL link for a PR, this fixes it

GuySartorelli · 2024-01-21T00:53:00Z

I've renamed the RunsWithoutProject annotation to CanRunGlobally.

This is a more immediately clear and better descriptive name for what this annotation does, and leaves its usage open to the behaviour described in #4416 (comment) in the future.

Also ran this with the correct devpod URL this time (thanks Randy) and it works as expected.

GuySartorelli · 2024-01-22T04:50:55Z

Buildkite ci failure looks unrelated

rfay · 2024-01-22T12:00:43Z

I invited you to buildkite so you can restart tests (and more easily examine them), also to ddev team on GitHub. I think that lets you restart GitHub tests.

GuySartorelli · 2024-01-22T20:02:00Z

Awesome, thank you! 💙

GuySartorelli · 2024-01-24T07:52:52Z

This should be good to go

stasadev · 2024-01-24T15:26:44Z

I will review this later this week.

stasadev

This is a great addition to DDEV!

Works as expected, thank you @GuySartorelli.

GuySartorelli requested review from a team as code owners January 17, 2024 09:52

GuySartorelli marked this pull request as draft January 17, 2024 09:52

github-actions bot added the enhancement label Jan 17, 2024

GuySartorelli mentioned this pull request Jan 17, 2024

Allow global shell commands/custom commands to be run from everywhere (not just inside a project) #4416

Closed

1 task

GuySartorelli force-pushed the 20240116_GuySartorelli_truly-global-commands branch 2 times, most recently from 48fe5a7 to 7304e4a Compare January 17, 2024 10:00

GuySartorelli commented Jan 17, 2024

View reviewed changes

GuySartorelli changed the title ~~feat: allow global host commands to be run from anywhere, fixes #4416~~ feat: allow global host commands to be run from anywhere, for #4416 Jan 17, 2024

GuySartorelli force-pushed the 20240116_GuySartorelli_truly-global-commands branch from 7304e4a to feaedb5 Compare January 20, 2024 07:13

GuySartorelli marked this pull request as ready for review January 20, 2024 07:15

rfay added a commit that referenced this pull request Jan 20, 2024

docs: fix gitpod PR link

e1018d5

Per #5702 (comment) we had a typo in the URL link for a PR, this fixes it

rfay mentioned this pull request Jan 20, 2024

docs: fix gitpod PR link #5719

Merged

feat: allow global host commands to be run from anywhere

7ed5340

GuySartorelli force-pushed the 20240116_GuySartorelli_truly-global-commands branch from feaedb5 to 7ed5340 Compare January 21, 2024 00:51

stasadev approved these changes Jan 26, 2024

View reviewed changes

stasadev merged commit 82925f2 into ddev:master Jan 26, 2024
21 checks passed

GuySartorelli deleted the 20240116_GuySartorelli_truly-global-commands branch January 26, 2024 21:20

GuySartorelli mentioned this pull request Jan 30, 2024

Add global --root and --project flags to allow ddev to run in any directory #2128

Open

rfay mentioned this pull request Feb 16, 2024

fix: do not create project network programmatically on ddev start, fixes #5810 #5848

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow global host commands to be run from anywhere, for #4416 #5702

feat: allow global host commands to be run from anywhere, for #4416 #5702

GuySartorelli commented Jan 17, 2024 •

edited

github-actions bot commented Jan 17, 2024 •

edited

GuySartorelli Jan 17, 2024

GuySartorelli Jan 17, 2024

GuySartorelli Jan 17, 2024 •

edited

GuySartorelli commented Jan 17, 2024

stasadev commented Jan 17, 2024

GuySartorelli commented Jan 20, 2024

GuySartorelli commented Jan 20, 2024

rfay commented Jan 20, 2024

rfay commented Jan 20, 2024

GuySartorelli commented Jan 21, 2024 •

edited

GuySartorelli commented Jan 22, 2024 •

edited

rfay commented Jan 22, 2024

GuySartorelli commented Jan 22, 2024

GuySartorelli commented Jan 24, 2024

stasadev commented Jan 24, 2024

stasadev left a comment

		// addCustomCommandsFromDir adds the custom commands from inside a given directory
		func addCustomCommandsFromDir(rootCmd cobra.Command, app ddevapp.DdevApp, serviceDirOnHost string, commandFiles []string, isGlobalSet bool, commandsAdded map[string]int) error {

feat: allow global host commands to be run from anywhere, for #4416 #5702

feat: allow global host commands to be run from anywhere, for #4416 #5702

Conversation

GuySartorelli commented Jan 17, 2024 • edited

The Issue

How This PR Solves The Issue

Manual Testing Instructions

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

github-actions bot commented Jan 17, 2024 • edited

GuySartorelli Jan 17, 2024

Choose a reason for hiding this comment

GuySartorelli Jan 17, 2024

Choose a reason for hiding this comment

GuySartorelli Jan 17, 2024 • edited

Choose a reason for hiding this comment

GuySartorelli commented Jan 17, 2024

stasadev commented Jan 17, 2024

GuySartorelli commented Jan 20, 2024

GuySartorelli commented Jan 20, 2024

rfay commented Jan 20, 2024

rfay commented Jan 20, 2024

GuySartorelli commented Jan 21, 2024 • edited

GuySartorelli commented Jan 22, 2024 • edited

rfay commented Jan 22, 2024

GuySartorelli commented Jan 22, 2024

GuySartorelli commented Jan 24, 2024

stasadev commented Jan 24, 2024

stasadev left a comment

Choose a reason for hiding this comment

GuySartorelli commented Jan 17, 2024 •

edited

github-actions bot commented Jan 17, 2024 •

edited

GuySartorelli Jan 17, 2024 •

edited

GuySartorelli commented Jan 21, 2024 •

edited

GuySartorelli commented Jan 22, 2024 •

edited