Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Warn users when GitHub Actions is making a merge commit #222

Merged
merged 9 commits into from
Aug 24, 2023
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ test:
test_setup:
mkdir -p ${CODE_PATH}
cd ${CODE_PATH} && ls -A1 | xargs rm -rf
git clone https://github.com/DeepSourceCorp/cli ${CODE_PATH}
git clone https://github.com/DeepSourceCorp/july ${CODE_PATH}
chmod +x /tmp/deepsource
cp ./command/report/tests/golden_files/python_coverage.xml /tmp

Expand Down
105 changes: 78 additions & 27 deletions command/report/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,31 @@ import (
)

// gitGetHead accepts a git directory and returns head commit OID / error
func gitGetHead(workspaceDir string) (string, error) {
func gitGetHead(workspaceDir string) (headOID string, warning string, err error) {
// Check if DeepSource's Test coverage action triggered this first before executing any git commands.
ghaHeadOID, err := getTestCoverageActionCommit()
if ghaHeadOID != "" {
return ghaHeadOID, warning, err
}

// get the top commit manually, using git command
headOID, err = fetchHeadManually(workspaceDir)
if err != nil {
fmt.Println(err)
sentry.CaptureException(err)
return "", "", err
sourya-deepsource marked this conversation as resolved.
Show resolved Hide resolved
}

// TRAVIS CI
// Get USER env variable.
if envUser := os.Getenv("USER"); envUser == "travis" {
// Travis creates a merge commit for pull requests on forks.
// The head of commit is this merge commit, which does not match the commit of deepsource check.
// Fetch value of pull request SHA. If this is a PR, it will return SHA of HEAD commit of the PR, else "".
// If prSHA is not empty, that means we got an SHA, which is HEAD. Return this.
if prSHA := os.Getenv("TRAVIS_PULL_REQUEST_SHA"); len(prSHA) > 0 {
return prSHA, nil
}
headOID, warning, err = getTravisCommit(headOID)
return
}

// GITHUB ACTIONS
// Check if it is a GitHub Action Environment
// If it is: then get the HEAD from `GITHUB_SHA` environment
if _, isGitHubEnv := os.LookupEnv("GITHUB_ACTIONS"); isGitHubEnv {
// When GITHUB_REF is not set, GITHUB_SHA points to original commit.
// When set, it points to the "latest *merge* commit in the branch".
// Ref: https://help.github.com/en/actions/reference/events-that-trigger-workflows#pull-request-event-pull_request
if _, isBranchCommit := os.LookupEnv("GITHUB_REF"); !isBranchCommit {
return os.Getenv("GITHUB_SHA"), nil
}
headOID, warning, err = getGitHubActionsCommit(headOID)
return
}

// Check if the `GIT_COMMIT_SHA` environment variable exists. If yes, return this as
Expand All @@ -47,18 +49,11 @@ func gitGetHead(workspaceDir string) (string, error) {
// GIT_COMMIT_SHA=$(git --no-pager rev-parse HEAD | tr -d '\n')
// docker run -e DEEPSOURCE_DSN -e GIT_COMMIT_SHA ...
if _, isManuallyInjectedSHA := os.LookupEnv("GIT_COMMIT_SHA"); isManuallyInjectedSHA {
return os.Getenv("GIT_COMMIT_SHA"), nil
return os.Getenv("GIT_COMMIT_SHA"), "", nil
}

// If we are here, it means this is neither GitHub Action on default branch,
// nor a travis env with PR. Continue to fetch the headOID via the git command.
headOID, err := fetchHeadManually(workspaceDir)
if err != nil {
fmt.Println(err)
sentry.CaptureException(err)
return "", err
}
return headOID, nil
// If we are here, it means there weren't any special cases. Return the manually found headOID.
return headOID, warning, nil
sourya-deepsource marked this conversation as resolved.
Show resolved Hide resolved
}

// Fetches the latest commit hash using the command `git rev-parse HEAD`
Expand All @@ -80,3 +75,59 @@ func fetchHeadManually(directoryPath string) (string, error) {
// Trim newline suffix from Commit OID
return strings.TrimSuffix(outStr, "\n"), nil
}

// Handle special cases for GitHub Actions.
func getGitHubActionsCommit(topCommit string) (headOID string, warning string, err error) {
// When GITHUB_REF is not set, GITHUB_SHA points to original commit.
// When set, it points to the "latest *merge* commit in the branch".
// Early exit when GITHUB_SHA points to the original commit.
// Ref: https://help.github.com/en/actions/reference/events-that-trigger-workflows#pull-request-event-pull_request
if _, isRefPresent := os.LookupEnv("GITHUB_REF"); !isRefPresent {
return os.Getenv("GITHUB_SHA"), "", nil
}

// Case: Detect Merge commit made by GitHub Actions, which pull_request events are nutorious to make.
// We are anyways going to return `headOID` fetched manually, but want to warn users about the merge commit.

// When ref is not provided during the checkout step, headOID would be the same as GITHUB_SHA
// This confirms the merge commit.
// event names where GITHUB_SHA would be of a merge commit:
// "pull_request",
// "pull_request_review",
// "pull_request_review",
// "pull_request_review_comment",
eventName := os.Getenv("GITHUB_EVENT_NAME")
eventCommitSha := os.Getenv("GITHUB_SHA")
if strings.HasPrefix(eventName, "pull_request") && headOID == eventCommitSha {
warning = "Warning: Looks like the checkout step is making a merge commit. " +
"Test coverage Analyzer would not run for the reported artifact because the merge commit doesn't exist upstream.\n" +
"Please refer to the docs for required changes. Ref: https://docs.deepsource.com/docs/analyzers-test-coverage#with-github-actions"
}

return topCommit, warning, err
}

// Return PR's HEAD ref set as env variable manually by DeepSource's Test coverage action.
func getTestCoverageActionCommit() (headOID string, err error) {
// This is kept separate from `getGitHubActionsCommit` because we don't want to run any git command manually
// before this is checked. Since this is guaranteed to be set if artifact is sent using our GitHub action,
// we can reliably send the commit SHA, and no git commands are executed, making the actions work all the time. \o/

// We are setting PR's head commit as default using github context as env variable: "GHA_HEAD_COMMIT_SHA"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

headOID = os.Getenv("GHA_HEAD_COMMIT_SHA")

return
}

// Handle special case for TravisCI
func getTravisCommit(topCommit string) (headOID string, warning string, err error) {
// Travis creates a merge commit for pull requests on forks.
// The head of commit is this merge commit, which does not match the commit of deepsource check.
// Fetch value of pull request SHA. If this is a PR, it will return SHA of HEAD commit of the PR, else "".
// If prSHA is not empty, that means we got an SHA, which is HEAD. Return this.
if prSHA := os.Getenv("TRAVIS_PULL_REQUEST_SHA"); len(prSHA) > 0 {
return prSHA, warning, err
}

return topCommit, warning, err
}
5 changes: 4 additions & 1 deletion command/report/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (opts *ReportOptions) Run() int {
dsnAccessToken := dsnSplitTokenHost[0]

// Head Commit OID
headCommitOID, err := gitGetHead(currentDir)
headCommitOID, warning, err := gitGetHead(currentDir)
if err != nil {
fmt.Fprintln(os.Stderr, "DeepSource | Error | Unable to get commit OID HEAD. Make sure you are running the CLI from a git repository")
sentry.CaptureException(err)
Expand Down Expand Up @@ -375,5 +375,8 @@ func (opts *ReportOptions) Run() int {
if queryResponse.Data.CreateArtifact.Message != "" {
fmt.Printf("Message %s\n", queryResponse.Data.CreateArtifact.Message)
}
if warning != "" {
fmt.Print(warning)
}
return 0
}
Loading
Loading