From 928972feeb5e4a7075b19074e713d3d40b5e6d4d Mon Sep 17 00:00:00 2001 From: Marco Ferrari Date: Fri, 2 Aug 2024 10:37:27 +0200 Subject: [PATCH] fix: adjust previous commit id on merge commits (#5950) - When a commit is a merge commit, adjust the revision so that it selects the correct parent. - Add relevant tests. - Extract the logic to initialize a Git repository for tests in a dedicated function. Fix #5941 --- Makefile | 9 +- lib/linter.sh | 25 +- .../github-event-push-merge-commit.json | 246 ++++++++++++++++++ test/run-super-linter-tests.sh | 59 ++++- 4 files changed, 328 insertions(+), 11 deletions(-) create mode 100644 test/data/github-event/github-event-push-merge-commit.json diff --git a/Makefile b/Makefile index 022290c2..fc6d0094 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ all: info docker test ## Run all targets. .PHONY: test -test: info validate-container-image-labels docker-build-check docker-dev-container-build-check test-lib inspec lint-codebase test-default-config-files test-actions-runner-debug test-actions-steps-debug test-runner-debug test-find lint-subset-files test-custom-ssl-cert test-non-default-workdir test-git-flags test-non-default-home-directory test-git-initial-commit test-log-level test-use-find-and-ignore-gitignored-files test-linters-expect-failure-log-level-notice test-bash-exec-library-expect-success test-bash-exec-library-expect-failure test-save-super-linter-output test-save-super-linter-output-custom-path test-save-super-linter-custom-summary test-linters ## Run the test suite +test: info validate-container-image-labels docker-build-check docker-dev-container-build-check test-lib inspec lint-codebase test-default-config-files test-actions-runner-debug test-actions-steps-debug test-runner-debug test-find lint-subset-files test-custom-ssl-cert test-non-default-workdir test-git-flags test-non-default-home-directory test-git-initial-commit test-git-merge-commit-push test-log-level test-use-find-and-ignore-gitignored-files test-linters-expect-failure-log-level-notice test-bash-exec-library-expect-success test-bash-exec-library-expect-failure test-save-super-linter-output test-save-super-linter-output-custom-path test-save-super-linter-custom-summary test-linters ## Run the test suite # if this session isn't interactive, then we don't want to allocate a # TTY, which would fail, but if it is interactive, we do want to attach @@ -425,6 +425,13 @@ test-git-initial-commit: ## Run super-linter against a repository that only has "run_test_case_git_initial_commit" \ "$(IMAGE)" +.PHONY: test-git-merge-commit-push +test-git-merge-commit-push: ## Run super-linter against a repository that has merge commits on a push event + $(CURDIR)/test/run-super-linter-tests.sh \ + $(SUPER_LINTER_TEST_CONTAINER_URL) \ + "run_test_case_merge_commit_push" \ + "$(IMAGE)" + .PHONY: test-use-find-and-ignore-gitignored-files test-use-find-and-ignore-gitignored-files: ## Run super-linter with USE_FIND_ALGORITHM=true and IGNORE_GITIGNORED_FILES=true $(CURDIR)/test/run-super-linter-tests.sh \ diff --git a/lib/linter.sh b/lib/linter.sh index 8bec30fd..b8b33dc7 100755 --- a/lib/linter.sh +++ b/lib/linter.sh @@ -410,8 +410,31 @@ GetGitHubVars() { # Ref: https://docs.github.com/en/actions/learn-github-actions/contexts#github-context debug "Get the hash of the commit to start the diff from from Git because the GitHub push event payload may not contain references to base_ref or previous commit." + + debug "Check if the commit is a merge commit by checking if it has more than one parent" + local GIT_COMMIT_PARENTS_COUNT + GIT_COMMIT_PARENTS_COUNT=$(git -C "${GITHUB_WORKSPACE}" rev-list --parents -n 1 "${GITHUB_SHA}" | wc -w) + debug "Git commit parents count (GIT_COMMIT_PARENTS_COUNT): ${GIT_COMMIT_PARENTS_COUNT}" + GIT_COMMIT_PARENTS_COUNT=$((GIT_COMMIT_PARENTS_COUNT - 1)) + debug "Subtract 1 from GIT_COMMIT_PARENTS_COUNT to get the actual number of merge parents because the count includes the commit itself. GIT_COMMIT_PARENTS_COUNT: ${GIT_COMMIT_PARENTS_COUNT}" + + # Ref: https://git-scm.com/docs/git-rev-parse#Documentation/git-rev-parse.txt + local GIT_BEFORE_SHA_HEAD="HEAD" + if [ ${GIT_COMMIT_PARENTS_COUNT} -gt 1 ]; then + debug "${GITHUB_SHA} is a merge commit because it has more than one parent." + GIT_BEFORE_SHA_HEAD="${GIT_BEFORE_SHA_HEAD}^2" + debug "Add the suffix to GIT_BEFORE_SHA_HEAD to get the second parent of the merge commit: ${GIT_BEFORE_SHA_HEAD}" + GITHUB_PUSH_COMMIT_COUNT=$((GITHUB_PUSH_COMMIT_COUNT - 1)) + debug "Remove one commit from GITHUB_PUSH_COMMIT_COUNT to account for the merge commit. GITHUB_PUSH_COMMIT_COUNT: ${GITHUB_PUSH_COMMIT_COUNT}" + else + debug "${GITHUB_SHA} is not a merge commit because it has a single parent. No need to add the parent identifier (^) to the revision indicator because it's implicitly set to ^1 when there's only one parent." + fi + + GIT_BEFORE_SHA_HEAD="${GIT_BEFORE_SHA_HEAD}~${GITHUB_PUSH_COMMIT_COUNT}" + debug "GIT_BEFORE_SHA_HEAD: ${GIT_BEFORE_SHA_HEAD}" + # shellcheck disable=SC2086 # We checked that GITHUB_PUSH_COMMIT_COUNT is an integer - if ! GITHUB_BEFORE_SHA=$(git -C "${GITHUB_WORKSPACE}" rev-parse HEAD~${GITHUB_PUSH_COMMIT_COUNT}); then + if ! GITHUB_BEFORE_SHA=$(git -C "${GITHUB_WORKSPACE}" rev-parse ${GIT_BEFORE_SHA_HEAD}); then fatal "Failed to initialize GITHUB_BEFORE_SHA for a push event. Output: ${GITHUB_BEFORE_SHA}" fi diff --git a/test/data/github-event/github-event-push-merge-commit.json b/test/data/github-event/github-event-push-merge-commit.json new file mode 100644 index 00000000..5dd4fac2 --- /dev/null +++ b/test/data/github-event/github-event-push-merge-commit.json @@ -0,0 +1,246 @@ +{ + "after": "cc62b30b91677b695d3929d6352e7bb541e5ed8c", + "base_ref": null, + "before": "dc27afaa1f4d1b098cbf7602b82de312d20fa481", + "commits": [ + { + "author": { + "email": "lae@lae.is", + "name": "lae", + "username": "lae" + }, + "committer": { + "email": "lae@lae.is", + "name": "lae", + "username": "lae" + }, + "distinct": false, + "id": "74e0339cc37cf77494b723107bd803f6adfd0d9f", + "message": "add README", + "timestamp": "2024-07-31T06:25:36+09:00", + "tree_id": "807fa49be3c439afd7723e7e3a8fea2103bd7368", + "url": "https://github.com/amplify-security/sl-repro-5941/commit/74e0339cc37cf77494b723107bd803f6adfd0d9f" + }, + { + "author": { + "email": "lae@lae.is", + "name": "lae", + "username": "lae" + }, + "committer": { + "email": "lae@lae.is", + "name": "lae", + "username": "lae" + }, + "distinct": false, + "id": "6a887dce059f8bb8bbe597770fd23f88478d6917", + "message": "update README", + "timestamp": "2024-07-31T06:28:50+09:00", + "tree_id": "d12d6bf3200d2bf95052ed41ad0f0c7c58c91dc2", + "url": "https://github.com/amplify-security/sl-repro-5941/commit/6a887dce059f8bb8bbe597770fd23f88478d6917" + }, + { + "author": { + "email": "lae@lae.is", + "name": "lae", + "username": "lae" + }, + "committer": { + "email": "lae@lae.is", + "name": "lae", + "username": "lae" + }, + "distinct": false, + "id": "03571b8e42247c5451b7d1cca3e2eccedb5bc4d3", + "message": "update README", + "timestamp": "2024-07-31T06:31:01+09:00", + "tree_id": "807fa49be3c439afd7723e7e3a8fea2103bd7368", + "url": "https://github.com/amplify-security/sl-repro-5941/commit/03571b8e42247c5451b7d1cca3e2eccedb5bc4d3" + }, + { + "author": { + "email": "lae@users.noreply.github.com", + "name": "lae", + "username": "lae" + }, + "committer": { + "email": "noreply@github.com", + "name": "GitHub", + "username": "web-flow" + }, + "distinct": true, + "id": "cc62b30b91677b695d3929d6352e7bb541e5ed8c", + "message": "Merge pull request #1 from amplify-security/feature/readme\nadd README", + "timestamp": "2024-07-31T06: 33: 25+09: 00", + "tree_id": "807fa49be3c439afd7723e7e3a8fea2103bd7368", + "url": "https://github.com/amplify-security/sl-repro-5941/commit/cc62b30b91677b695d3929d6352e7bb541e5ed8c" + } + ], + "compare": "https://github.com/amplify-security/sl-repro-5941/compare/dc27afaa1f4d...cc62b30b9167", + "created": false, + "deleted": false, + "forced": false, + "head_commit": { + "author": { + "email": "lae@users.noreply.github.com", + "name": "lae", + "username": "lae" + }, + "committer": { + "email": "noreply@github.com", + "name": "GitHub", + "username": "web-flow" + }, + "distinct": true, + "id": "cc62b30b91677b695d3929d6352e7bb541e5ed8c", + "message": "Merge pull request #1 from amplify-security/feature/readme\nadd README", + "timestamp": "2024-07-31T06: 33: 25+09: 00", + "tree_id": "807fa49be3c439afd7723e7e3a8fea2103bd7368", + "url": "https://github.com/amplify-security/sl-repro-5941/commit/cc62b30b91677b695d3929d6352e7bb541e5ed8c" + }, + "organization": { + "avatar_url": "https://avatars.githubusercontent.com/u/105519894?v=4", + "description": "", + "events_url": "https://api.github.com/orgs/amplify-security/events", + "hooks_url": "https://api.github.com/orgs/amplify-security/hooks", + "id": 105519894, + "issues_url": "https://api.github.com/orgs/amplify-security/issues", + "login": "amplify-security", + "members_url": "https://api.github.com/orgs/amplify-security/members{/member}", + "node_id": "O_kgDOBkobFg", + "public_members_url": "https://api.github.com/orgs/amplify-security/public_members{/member}", + "repos_url": "https://api.github.com/orgs/amplify-security/repos", + "url": "https://api.github.com/orgs/amplify-security" + }, + "pusher": { + "email": "lae@users.noreply.github.com", + "name": "lae" + }, + "ref": "refs/heads/main", + "repository": { + "allow_forking": true, + "archive_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/{archive_format}{/ref}", + "archived": false, + "assignees_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/assignees{/user}", + "blobs_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/git/blobs{/sha}", + "branches_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/branches{/branch}", + "clone_url": "https://github.com/amplify-security/sl-repro-5941.git", + "collaborators_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/collaborators{/collaborator}", + "comments_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/comments{/number}", + "commits_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/commits{/sha}", + "compare_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/compare/{base}...{head}", + "contents_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/contents/{+path}", + "contributors_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/contributors", + "created_at": 1722374504, + "custom_properties": {}, + "default_branch": "main", + "deployments_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/deployments", + "description": null, + "disabled": false, + "downloads_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/downloads", + "events_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/events", + "fork": false, + "forks": 0, + "forks_count": 0, + "forks_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/forks", + "full_name": "amplify-security/sl-repro-5941", + "git_commits_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/git/commits{/sha}", + "git_refs_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/git/refs{/sha}", + "git_tags_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/git/tags{/sha}", + "git_url": "git://github.com/amplify-security/sl-repro-5941.git", + "has_discussions": false, + "has_downloads": true, + "has_issues": true, + "has_pages": false, + "has_projects": true, + "has_wiki": true, + "homepage": null, + "hooks_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/hooks", + "html_url": "https://github.com/amplify-security/sl-repro-5941", + "id": 835952240, + "is_template": false, + "issue_comment_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/issues/comments{/number}", + "issue_events_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/issues/events{/number}", + "issues_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/issues{/number}", + "keys_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/keys{/key_id}", + "labels_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/labels{/name}", + "language": null, + "languages_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/languages", + "license": null, + "master_branch": "main", + "merges_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/merges", + "milestones_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/milestones{/number}", + "mirror_url": null, + "name": "sl-repro-5941", + "node_id": "R_kgDOMdOecA", + "notifications_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/notifications{?since,all,participating}", + "open_issues": 0, + "open_issues_count": 0, + "organization": "amplify-security", + "owner": { + "avatar_url": "https://avatars.githubusercontent.com/u/105519894?v=4", + "email": null, + "events_url": "https://api.github.com/users/amplify-security/events{/privacy}", + "followers_url": "https://api.github.com/users/amplify-security/followers", + "following_url": "https://api.github.com/users/amplify-security/following{/other_user}", + "gists_url": "https://api.github.com/users/amplify-security/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/amplify-security", + "id": 105519894, + "login": "amplify-security", + "name": "amplify-security", + "node_id": "O_kgDOBkobFg", + "organizations_url": "https://api.github.com/users/amplify-security/orgs", + "received_events_url": "https://api.github.com/users/amplify-security/received_events", + "repos_url": "https://api.github.com/users/amplify-security/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/amplify-security/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/amplify-security/subscriptions", + "type": "Organization", + "url": "https://api.github.com/users/amplify-security" + }, + "private": false, + "pulls_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/pulls{/number}", + "pushed_at": 1722375205, + "releases_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/releases{/id}", + "size": 0, + "ssh_url": "git@github.com:amplify-security/sl-repro-5941.git", + "stargazers": 0, + "stargazers_count": 0, + "stargazers_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/stargazers", + "statuses_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/statuses/{sha}", + "subscribers_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/subscribers", + "subscription_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/subscription", + "svn_url": "https://github.com/amplify-security/sl-repro-5941", + "tags_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/tags", + "teams_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/teams", + "topics": [], + "trees_url": "https://api.github.com/repos/amplify-security/sl-repro-5941/git/trees{/sha}", + "updated_at": "2024-07-30T21:24:53Z", + "url": "https://github.com/amplify-security/sl-repro-5941", + "visibility": "public", + "watchers": 0, + "watchers_count": 0, + "web_commit_signoff_required": false + }, + "sender": { + "avatar_url": "https://avatars.githubusercontent.com/u/1318013?v=4", + "events_url": "https://api.github.com/users/lae/events{/privacy}", + "followers_url": "https://api.github.com/users/lae/followers", + "following_url": "https://api.github.com/users/lae/following{/other_user}", + "gists_url": "https://api.github.com/users/lae/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/lae", + "id": 1318013, + "login": "lae", + "node_id": "MDQ6VXNlcjEzMTgwMTM=", + "organizations_url": "https://api.github.com/users/lae/orgs", + "received_events_url": "https://api.github.com/users/lae/received_events", + "repos_url": "https://api.github.com/users/lae/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/lae/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/lae/subscriptions", + "type": "User", + "url": "https://api.github.com/users/lae" + } +} diff --git a/test/run-super-linter-tests.sh b/test/run-super-linter-tests.sh index 41eeb4c8..9f268204 100755 --- a/test/run-super-linter-tests.sh +++ b/test/run-super-linter-tests.sh @@ -62,33 +62,74 @@ run_test_case_bash_exec_library_expect_success() { COMMAND_TO_RUN+=(-e BASH_EXEC_IGNORE_LIBRARIES="true") } -run_test_case_git_initial_commit() { - local GIT_REPOSITORY_PATH - GIT_REPOSITORY_PATH="$(mktemp -d)" +initialize_git_repository_and_test_args() { + local GIT_REPOSITORY_PATH="${1}" # shellcheck disable=SC2064 # Once the path is set, we don't expect it to change trap "rm -fr '${GIT_REPOSITORY_PATH}'" EXIT + local GITHUB_EVENT_FILE_PATH="${2}" + git -C "${GIT_REPOSITORY_PATH}" init --initial-branch="${DEFAULT_BRANCH}" git -C "${GIT_REPOSITORY_PATH}" config user.name "Super-linter Test" git -C "${GIT_REPOSITORY_PATH}" config user.email "super-linter-test@example.com" - cp -v test/data/github-event/github-event-push.json "${GIT_REPOSITORY_PATH}/" + # Put an arbitrary JSON file in the repository to trigger some validation + cp -v "${GITHUB_EVENT_FILE_PATH}" "${GIT_REPOSITORY_PATH}/" git -C "${GIT_REPOSITORY_PATH}" add . git -C "${GIT_REPOSITORY_PATH}" commit -m "feat: initial commit" - local TEST_GITHUB_SHA - TEST_GITHUB_SHA="$(git -C "${GIT_REPOSITORY_PATH}" rev-parse HEAD)" - RUN_LOCAL=false SUPER_LINTER_WORKSPACE="${GIT_REPOSITORY_PATH}" COMMAND_TO_RUN+=(-e GITHUB_WORKSPACE="/tmp/lint") COMMAND_TO_RUN+=(-e GITHUB_EVENT_NAME="push") - COMMAND_TO_RUN+=(-e GITHUB_EVENT_PATH="/tmp/lint/github-event-push.json") - COMMAND_TO_RUN+=(-e GITHUB_SHA="${TEST_GITHUB_SHA}") + COMMAND_TO_RUN+=(-e GITHUB_EVENT_PATH="/tmp/lint/$(basename "${GITHUB_EVENT_FILE_PATH}")") COMMAND_TO_RUN+=(-e MULTI_STATUS=false) COMMAND_TO_RUN+=(-e VALIDATE_ALL_CODEBASE=false) COMMAND_TO_RUN+=(-e VALIDATE_JSON=true) } +run_test_case_git_initial_commit() { + local GIT_REPOSITORY_PATH + GIT_REPOSITORY_PATH="$(mktemp -d)" + + initialize_git_repository_and_test_args "${GIT_REPOSITORY_PATH}" "test/data/github-event/github-event-push.json" + + local TEST_GITHUB_SHA + TEST_GITHUB_SHA="$(git -C "${GIT_REPOSITORY_PATH}" rev-parse HEAD)" + COMMAND_TO_RUN+=(-e GITHUB_SHA="${TEST_GITHUB_SHA}") +} + +run_test_case_merge_commit_push() { + local GIT_REPOSITORY_PATH + GIT_REPOSITORY_PATH="$(mktemp -d)" + + initialize_git_repository_and_test_args "${GIT_REPOSITORY_PATH}" "test/data/github-event/github-event-push-merge-commit.json" + + local NEW_BRANCH_NAME="branch-1" + git -C "${GIT_REPOSITORY_PATH}" switch --create "${NEW_BRANCH_NAME}" + cp -v "test/data/github-event/github-event-push-merge-commit.json" "${GIT_REPOSITORY_PATH}/new-file-1.json" + git -C "${GIT_REPOSITORY_PATH}" add . + git -C "${GIT_REPOSITORY_PATH}" commit -m "feat: add new file 1" + cp -v "test/data/github-event/github-event-push-merge-commit.json" "${GIT_REPOSITORY_PATH}/new-file-2.json" + git -C "${GIT_REPOSITORY_PATH}" add . + git -C "${GIT_REPOSITORY_PATH}" commit -m "feat: add new file 2" + cp -v "test/data/github-event/github-event-push-merge-commit.json" "${GIT_REPOSITORY_PATH}/new-file-3.json" + git -C "${GIT_REPOSITORY_PATH}" add . + git -C "${GIT_REPOSITORY_PATH}" commit -m "feat: add new file 3" + git -C "${GIT_REPOSITORY_PATH}" switch "${DEFAULT_BRANCH}" + # Force the creation of a merge commit + git -C "${GIT_REPOSITORY_PATH}" merge \ + -m "Merge commit" \ + --no-ff \ + "${NEW_BRANCH_NAME}" + git -C "${GIT_REPOSITORY_PATH}" branch -d "${NEW_BRANCH_NAME}" + + git -C "${GIT_REPOSITORY_PATH}" log --all --graph --abbrev-commit --decorate --format=oneline + + local TEST_GITHUB_SHA + TEST_GITHUB_SHA="$(git -C "${GIT_REPOSITORY_PATH}" rev-parse HEAD)" + COMMAND_TO_RUN+=(-e GITHUB_SHA="${TEST_GITHUB_SHA}") +} + run_test_case_use_find_and_ignore_gitignored_files() { ignore_test_cases COMMAND_TO_RUN+=(-e IGNORE_GITIGNORED_FILES=true)