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

Correct getting affected projects in Nx #2

Merged
merged 6 commits into from
Sep 3, 2024
Merged
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
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# merge-action
# nx-action

Hosts the code for the [Trunk Merge Queue](https://docs.trunk.io/merge-queue) GitHub Action, which
makes it easy to upload the required impacted targets for PRs when running your merge queues in
Expand Down Expand Up @@ -95,9 +95,9 @@ share an impacted target must be tested together; otherwise, they can be tested

## Under the hood

We use the Nx CLI to create a dependency graph that shows all of the affected libraries from the
pull request. More information on the command can be found
[here](https://nx.dev/nx-api/nx/documents/dep-graph)
We use the Nx CLI to show all affected projects betweent the current branch and the tip of main.
More information on the command
[can be found here](https://nx.dev/nx-api/nx/documents/show#affected)

## Questions

Expand Down
6 changes: 3 additions & 3 deletions action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ inputs:
required: false
workspace-path:
description: Path to your Nx workspace. If none is provided, will use the root directory.
required: true
required: false
impact-all-filters-path:
description:
A path to a list of filters to identify whether `ALL` impacted targets should be considered.
Expand Down Expand Up @@ -60,7 +60,7 @@ runs:
- name: Compute Impacted Targets
if: ${{ steps.prerequisites.outputs.impacts_all_detected == 'false' }}
id: compute-impacted-targets
working-directory: ${{ inputs.workspace-path }}
working-directory: ${{ inputs.workspace-path || '.' }}
run: python ${GITHUB_ACTION_PATH}/src/compute_impacted_targets.py
shell: bash
env:
Expand All @@ -73,7 +73,7 @@ runs:

- name: Upload Impacted Targets
id: upload-impacted-targets
working-directory: ${{ inputs.workspace-path }}
working-directory: ${{ inputs.workspace-path || '.' }}
run: python ${GITHUB_ACTION_PATH}/src/upload_impacted_targets.py
shell: bash
env:
Expand Down
27 changes: 11 additions & 16 deletions src/compute_impacted_targets.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import json
import os

from utils import (
Expand Down Expand Up @@ -27,33 +26,29 @@ def log_if_verbose(log=""):

# Get the list of impacted targets by leveraging Nx's dependency graph capabilities.
# https://nx.dev/nx-api/nx/documents/dep-graph
affected_json_out = f"./{merge_instance_branch_head_sha}_{pr_branch_head_sha}.json"
nx_graph_command_base = f"npx nx graph --affected --verbose --base={merge_instance_branch_head_sha} --head={pr_branch_head_sha}"
graph_output = run_command(
f"{nx_graph_command_base} --file={affected_json_out}", verbose=verbose
)
log_if_verbose(graph_output)

affected_json = json.loads(run_command(f"cat {affected_json_out}", return_output=True))
affected_list_out = f"./{merge_instance_branch_head_sha}_{pr_branch_head_sha}.txt"
nx_show_command_base = f"npx nx show projects --affected --base={merge_instance_branch_head_sha} --head={pr_branch_head_sha}"
Copy link

Choose a reason for hiding this comment

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

Tested this out locally and it seems to check out.

affected_output = run_command(nx_show_command_base, verbose=verbose, return_output=True)

impacted_projects = (
affected_json["affectedProjects"] if "affectedProjects" in affected_json else []
)
print(f"Impacted projects are:")
print(*impacted_projects, sep=",\n")
print(affected_output)

affected_projects = []
if affected_output:
affected_projects = affected_output.split("\n")

# Move this to a file so we can pass it to the next action, as this list
# can be rather large.
impacted_targets_out = f"./{merge_instance_branch_head_sha}"
with open(impacted_targets_out, "w", encoding="utf-8") as f:
f.write(f"{impacted_projects}")
f.write(f"{affected_output}")

num_impacted_projects = len(impacted_projects)
num_impacted_projects = len(affected_projects)
print(
f"Computed {num_impacted_projects} impacted projects for sha {pr_branch_head_sha}"
)

print(f"To replicate this command, run the following:\n{nx_graph_command_base}\n")
print(f"To replicate this command, run the following:\n{nx_show_command_base}\n")

# Outputs
github_output = f"impacted_targets_out={impacted_targets_out}\n"
Expand Down
5 changes: 3 additions & 2 deletions src/prerequisites.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ def fetch_remote_git_history(ref):
print("No merge instance branch found. Exiting.")
sys.exit(1)

write_to_github_output(f"merge_instance_branch={merge_instance_branch}")
Copy link

Choose a reason for hiding this comment

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

This is just logging, essentially?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This, which is called during the prerequisite step of this workflow, is outputting to github's output, which gets passed around between all of the remaining steps in the flow. That's why we switch to passing in steps.prerequisites.outputs.merge_instance_branch for the merge branch for the other steps (link). We do this because providing the merge branch to us is optional - we use the repo's default branch if it isn't provided, and we make the prerequisite step figure that out for us (link)


# Check if any file specified by the filters in impact-all-filters-path was
# changed in the PR. If it was, then mark this PR as impacting all other PRs.
impacts_filters_changes = os.environ.get("IMPACTS_FILTERS_CHANGES")
Expand All @@ -59,7 +61,6 @@ def fetch_remote_git_history(ref):
print("Could not identify merge instance branch head sha")
sys.exit(1)

github_output = f"merge_instance_branch={merge_instance_branch}\nmerge_instance_branch_head_sha={merge_instance_branch_head_sha}\nimpacts_all_detected=false"
log_if_verbose(f"Setting these outputs:\n{github_output}")
github_output = f"merge_instance_branch_head_sha={merge_instance_branch_head_sha}\nimpacts_all_detected=false"

write_to_github_output(github_output)
11 changes: 7 additions & 4 deletions src/upload_impacted_targets.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import ast
import json
import os
import sys
Expand All @@ -9,7 +8,7 @@

verbose = get_bool_from_string(os.environ.get("VERBOSE"))

IMPACTS_ALL_KEYWORD = "IMPACTS_ALL"
IMPACTS_ALL_KEYWORD = "ALL"


def log_if_verbose(log=""):
Expand Down Expand Up @@ -51,7 +50,7 @@ def log_if_verbose(log=""):
pr_number = get_and_require_env_var("PR_NUMBER")
pr_branch_head_sha = get_and_require_env_var("PR_BRANCH_HEAD_SHA")

IMPACTED_TARGETS = ""
IMPACTED_TARGETS: str | list[str] = ""
impacts_all_detected = get_bool_from_string(
get_and_require_env_var("IMPACTS_ALL_DETECTED")
)
Expand All @@ -60,7 +59,11 @@ def log_if_verbose(log=""):
else:
impacted_targets_file = get_and_require_env_var("IMPACTED_TARGETS_FILE")
with open(impacted_targets_file, "r", encoding="utf-8") as f:
IMPACTED_TARGETS = ast.literal_eval(f.read())
content = f.read().strip()
if not content:
IMPACTED_TARGETS = []
else:
IMPACTED_TARGETS = content.split("\n")

log_if_verbose(f"Read impacted targets: {IMPACTED_TARGETS}")

Expand Down
2 changes: 1 addition & 1 deletion src/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@ def get_bool_from_string(bool_string):

def write_to_github_output(github_output):
with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as f:
f.write(github_output)
f.write(f"{github_output}\n")