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.

Sign up for GitHub

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

Jump to bottom

Allow code_style.py to work from a git hook #9233

Merged
merged 2 commits into from
Jun 7, 2024
Merged

Allow code_style.py to work from a git hook #9233

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
330680e
Allow code_style.py to work from a git hook
davidhorstmann-arm Jun 6, 2024
f8bbc2d
Remove multi-type variable
davidhorstmann-arm Jun 6, 2024
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
24 changes: 21 additions & 3 deletions scripts/code_style.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,24 @@ def get_src_files(since: Optional[str]) -> List[str]:
output = subprocess.check_output(["git", "ls-files"] + file_patterns,
universal_newlines=True)
src_files = output.split()

# When this script is called from a git hook, some environment variables
# are set by default which force all git commands to use the main repository
# (i.e. prevent us from performing commands on the framework repo).
# Create an environment without these variables for running commands on the
# framework repo.
framework_env = os.environ.copy()
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a comment that we may want to do that in check_files.py as well. Not necessarily in this PR though.

# Get a list of environment vars that git sets
git_env_vars = subprocess.check_output(["git", "rev-parse", "--local-env-vars"],
universal_newlines=True)
# Remove the vars from the environment
for var in git_env_vars.split():
framework_env.pop(var, None)

output = subprocess.check_output(["git", "-C", "framework", "ls-files"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the solution here is actually on the wrong track, although it's an improvement so I won't block it. When this script is called with --since, it looks for the given revision name in the submodule. Reported as Mbed-TLS/mbedtls-framework#24. I suspect the right solution would be to never call git -C framework and instead rely on --recurse-submodules throughout.

Copy link
Contributor

Choose a reason for hiding this comment

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

Using --recurse-submodules comes with some issues as well: see 7661aa0 commit message.

+ file_patterns, universal_newlines=True)
+ file_patterns,
universal_newlines=True,
env=framework_env)
framework_src_files = output.split()

if since:
Expand All @@ -89,7 +105,8 @@ def get_src_files(since: Optional[str]) -> List[str]:
# ... the framework submodule
cmd = ["git", "-C", "framework", "log", since + "..HEAD",
"--name-only", "--pretty=", "--"] + framework_src_files
output = subprocess.check_output(cmd, universal_newlines=True)
output = subprocess.check_output(cmd, universal_newlines=True,
env=framework_env)
committed_changed_files += ["framework/" + s for s in output.split()]

# and also get all files with uncommitted changes in ...
Expand All @@ -100,7 +117,8 @@ def get_src_files(since: Optional[str]) -> List[str]:
# ... the framework submodule
cmd = ["git", "-C", "framework", "diff", "--name-only", "--"] + \
framework_src_files
output = subprocess.check_output(cmd, universal_newlines=True)
output = subprocess.check_output(cmd, universal_newlines=True,
env=framework_env)
uncommitted_changed_files += ["framework/" + s for s in output.split()]

src_files = committed_changed_files + uncommitted_changed_files
Expand Down