Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Dockerfile.dev
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
FROM golang:1.20
FROM golang:1.21

RUN go install gotest.tools/gotestsum@v1.12.2
RUN git config --global --add safe.directory /app

WORKDIR /app
34 changes: 34 additions & 0 deletions cmd/get.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"encoding/json"
"fmt"
"log"
"os"
Expand Down Expand Up @@ -283,6 +284,36 @@ func getAllAgents(client client.AgentApiV1AlphaApi, agentType string) ([]models.
return agents, nil
}

var GetCurrentProjectCmd = &cobra.Command{
Use: "current_project",
Short: "Get project for current directory",
Aliases: []string{"cur"},
Long: `Determine the Semaphore project associated with this repository.

Resolution order:
1. A remote selected by 'gh repo set-default', if present.
2. The 'origin' remote, when configured.
3. The 'upstream' remote, when configured.
4. Any remaining remote whose URL is shared by all candidates.
5. An explicit error when multiple distinct URLs remain.`,
Comment on lines +291 to +298
Copy link

Choose a reason for hiding this comment

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

nit: this long string has whitespaces before each line which I think wasn't intentional, please correct me if I'm wrong.


Run: func(cmd *cobra.Command, args []string) {
project, err := utils.InferProject()
utils.Check(err)

doJSON, err := cmd.Flags().GetBool("json")
utils.Check(err)
if doJSON {
jsonBody, err := json.MarshalIndent(project, "", " ")
utils.Check(err)
fmt.Println(string(jsonBody))
} else {
fmt.Println(project.Metadata.Id, project.Metadata.Name, project.Spec.Repository.Url)
Copy link

Choose a reason for hiding this comment

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

nit: we could format this in some way that is easy to split the string

}

},
}

var GetProjectCmd = &cobra.Command{
Use: "projects [name]",
Short: "Get projects.",
Expand Down Expand Up @@ -531,6 +562,9 @@ func init() {
getCmd.AddCommand(GetProjectCmd)
getCmd.AddCommand(GetAgentTypeCmd)

getCmd.AddCommand(GetCurrentProjectCmd)
GetCurrentProjectCmd.Flags().Bool("json", false, "print project information as json")

GetAgentsCmd.Flags().StringP("agent-type", "t", "",
"agent type; if specified, returns only agents for this agent type")
getCmd.AddCommand(GetAgentsCmd)
Expand Down
161 changes: 137 additions & 24 deletions cmd/utils/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,50 @@ package utils

import (
"fmt"
"slices"

"log"
"os/exec"
"strings"

"github.com/semaphoreci/cli/api/client"
"github.com/semaphoreci/cli/api/models"
)

type GitRemote struct {
Name string
URL string
Project models.ProjectV1Alpha
}

type GitRemoteList []GitRemote

func (grl GitRemoteList) Contains(remoteNameOrUrl string) bool {
Copy link

Choose a reason for hiding this comment

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

I couldn't find anywhere where this method is used, if this is a leftover please remove or tell where it is/will be used.

for _, gitRemote := range grl {
if gitRemote.Name == remoteNameOrUrl || gitRemote.URL == remoteNameOrUrl {
return true
}
}
return false
}

func (grl GitRemoteList) Get(remoteNameOrUrl string) (*GitRemote, error) {
for _, gitRemote := range grl {
if gitRemote.Name == remoteNameOrUrl || gitRemote.URL == remoteNameOrUrl {
return &gitRemote, nil
}
}
return &GitRemote{}, fmt.Errorf("no remote matching %s found in remote list")
Copy link

Choose a reason for hiding this comment

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

You are missing the parameter for the fmt.Errorf, I think it would be remoteNameOrUrl

}

func (grl GitRemoteList) URLs() []string {
urls := []string{}
for _, gitRemote := range grl {
urls = append(urls, gitRemote.URL)
}
return urls
}

func GetProjectId(name string) string {
projectClient := client.NewProjectV1AlphaApi()
project, err := projectClient.GetProject(name)
Expand All @@ -20,47 +56,79 @@ func GetProjectId(name string) string {
}

func InferProjectName() (string, error) {
originUrl, err := getGitOriginUrl()
project, err := InferProject()
if err != nil {
return "", err
}
return project.Metadata.Name, nil
}

log.Printf("Origin url: '%s'\n", originUrl)

projectName, err := getProjectIdFromUrl(originUrl)
func InferProject() (models.ProjectV1Alpha, error) {
// Note that getAllGitRemotesAndProjects will only return remotes
// where the URL of that remote is configured in a project we got
// from the API, so this list is a list of remotes valid projects
// configured. All we have to do now is pick one.
gitRemotes, err := getAllGitRemotesAndProjects()
if err != nil {
return "", err
return models.ProjectV1Alpha{}, err
}

return projectName, nil
}
// If the user is using GitHub and has run `gh repo set-default`, then
// we can get that 'base' remote name and see if we have a project for
// it.
ghBaseRemoteName, err := getGitHubBaseRemoteName()
if err != nil {
log.Printf("tried looking for a `gh` base repo configuration, but found none.")
} else {
gitRemote, err := gitRemotes.Get(ghBaseRemoteName)
if err == nil {
return gitRemote.Project, nil
}
}

func getProjectIdFromUrl(url string) (string, error) {
projectClient := client.NewProjectV1AlphaApi()
projects, err := projectClient.ListProjects()
// If we only got one remote with a configured project, return it;
// alternately, if we got multiple, return the alphabetically first
// one.
if len(gitRemotes) == 1 {
return gitRemotes[0].Project, nil
}
Comment on lines +89 to +94
Copy link

Choose a reason for hiding this comment

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

Can you explain to me this comment please, how is the gitRemotes sorted?


if err != nil {
return "", fmt.Errorf("getting project list failed '%s'", err)
// If we got an "origin" remote or an "upstream" remote, return that (in
// that order of preference)
for _, remoteName := range []string{"origin", "upstream"} {
remote, err := gitRemotes.Get(remoteName)
if err == nil {
return remote.Project, nil
}
}

projectName := ""
for _, p := range projects.Projects {
if p.Spec.Repository.Url == url {
projectName = p.Metadata.Name
break
// At this point, we have multiple remotes configured, all of which have
// a project configured in Semaphore, none of which are named "origin" or
// "upstream", and none of which are set as the gh base repo. The *most likely*
// explanation here is that the user has the same repo URL configured multiple
// times, or they're doing something extremely unusual. I'm not sure we can
// make the correct decision here.
allUrls := []string{}
for _, url := range gitRemotes.URLs() {
if !slices.Contains(allUrls, url) {
allUrls = append(allUrls, url)
}
}
Comment on lines +111 to 116
Copy link

Choose a reason for hiding this comment

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

This code is to remove duplicates right? This current implementation is O(n²) we could use a map here very easily


if projectName == "" {
return "", fmt.Errorf("project with url '%s' not found in this org", url)
// Okay, there's only one URL so we can just pick the relevant project.
if len(allUrls) == 1 {
return gitRemotes[0].Project, nil
}

return projectName, nil
// At this point we'd just be guessing, so let's give up
return models.ProjectV1Alpha{}, fmt.Errorf("found %d remotes with %d different URLs but cannot determine the correct one", len(gitRemotes), len(allUrls))
}

func getGitOriginUrl() (string, error) {
args := []string{"config", "remote.origin.url"}

// getGitHubBaseRemoteName checks to see if the `gh` cli tool has set a default
// remote for this repository. If not, or if we're not using Github at all, we
// can just ignore the error.
func getGitHubBaseRemoteName() (string, error) {
args := []string{"config", "--local", "--get-regexp", "gh-resolved"}
cmd := exec.Command("git", args...)
out, err := cmd.CombinedOutput()
if err != nil {
Expand All @@ -69,5 +137,50 @@ func getGitOriginUrl() (string, error) {
return "", fmt.Errorf("%s failed with message: '%s'\n%s", cmd_string, err, user_msg)
}

return strings.TrimSpace(string(out)), nil
lines := strings.Split(strings.TrimSpace(string(out)), "\n")

if len(lines) == 0 {
return "", fmt.Errorf("no GitHub base remote configured for this repository")
}
if len(lines) > 1 {
return "", fmt.Errorf("got multiple lines when looking for GitHub base remote")
}

fields := strings.Fields(lines[0])
remoteName := strings.Split(fields[0], ".")[1]
Copy link

Choose a reason for hiding this comment

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

You are accessing the position 1 of the array without checking the length of the array. Please do the same thing you did with line (split+len)

return remoteName, nil
}

func getAllGitRemotesAndProjects() (GitRemoteList, error) {
args := []string{"config", "--local", "--get-regexp", "remote\\..*\\.url"}
cmd := exec.Command("git", args...)
out, err := cmd.CombinedOutput()
if err != nil {
cmd_string := fmt.Sprintf("'%s %s'", "git", strings.Join(args, " "))
user_msg := "You are probably not in a git directory?"
return GitRemoteList{}, fmt.Errorf("%s failed with message: '%s'\n%s", cmd_string, err, user_msg)
}
lines := strings.Split(strings.TrimSpace(string(out)), "\n")

projectClient := client.NewProjectV1AlphaApi()
projects, err := projectClient.ListProjects()
Copy link

Choose a reason for hiding this comment

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

You are missing an error check here


remotes := GitRemoteList{}

for _, line := range lines {
fields := strings.Fields(line)
keyFields := strings.Split(line, ".")
remoteName := keyFields[1]
Copy link

Choose a reason for hiding this comment

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

Here we have the same potential problem as well, please add a length check.

url := fields[1]
Comment on lines +171 to +174
Copy link

Choose a reason for hiding this comment

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

To improve readability and validation we could extract this to a function and do some more validation of the expected formatting of the string that we expect.

But I'm okay with maintaining this style and just fixing the length check.


for _, proj := range projects.Projects {
if proj.Spec.Repository.Url == url {
remotes = append(remotes, GitRemote{Name: remoteName, URL: url, Project: proj})
break
}
}

}
Comment on lines +170 to +183
Copy link

Choose a reason for hiding this comment

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

This part of the code is O(n*m) because of the nested loops, you could use maps to improve the complexity to O(n+m).

I think this is not that much about performance but for readability. But I can see some cases that a few ms can compound in the long run.


return remotes, nil
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/semaphoreci/cli

go 1.20
go 1.21

require (
github.com/ghodss/yaml v1.0.0
Expand Down