Skip to content

fix: fallback to ssh if https fails and vice versa#2355

Merged
FabianKramm merged 1 commit intodevspace-sh:mainfrom
pratikjagrut:dependency.path
Oct 3, 2022
Merged

fix: fallback to ssh if https fails and vice versa#2355
FabianKramm merged 1 commit intodevspace-sh:mainfrom
pratikjagrut:dependency.path

Conversation

@pratikjagrut
Copy link
Contributor

What issue type does this pull request address? (keep at least one, remove the others)
/kind bugfix

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible)
resolves #2323

Please provide a short message that should be published in the DevSpace release notes
Fixed an issue where DevSpace hangs till git credentials are not provided.

Copy link
Collaborator

@lizardruss lizardruss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

I had some questions that might prompt changes, but otherwise it looks good.


args = append(args, options.Args...)
out, err := command.CombinedOutput(ctx, gr.LocalPath, expand.ListEnviron(os.Environ()...), "git", args...)
os.Setenv("GIT_SSH_COMMAND", "ssh -oBatchMode=yes")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this to prevent prompting for a password? If not, a comment with explanation might be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}
gitEnv = append(gitEnv, os.Environ()...)
out, err := command.CombinedOutput(ctx, gr.LocalPath, expand.ListEnviron(gitEnv...), "git", args...)
os.Unsetenv("GIT_SSH_COMMAND")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is os.Setenv() and os.Unsetenv() required, or is passing gitEnv when running the command sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is, I seem to misunderstand it.

@pratikjagrut pratikjagrut force-pushed the dependency.path branch 2 times, most recently from d883cc3 to 9d1564c Compare October 3, 2022 09:23
@FabianKramm FabianKramm merged commit db75233 into devspace-sh:main Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fallback to ssh if git https doesn't work

3 participants