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

Bad format #916

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
035d412
Produce DWARF4 by default
kwk Mar 24, 2022
deb354c
[workflows] Split pr-code-format into two parts to make it more secure
tstellar Jan 14, 2024
1122d45
Add pull-requests: write permission
tstellar Jan 14, 2024
e20f196
Process multiple comments
tstellar Jan 14, 2024
a16f663
XXX: Fix
tstellar Jan 15, 2024
acdf333
XXX: Debug
tstellar Jan 14, 2024
e93db24
Break formatting
tstellar Jan 14, 2024
025a076
Run code helper from checkouted out code
tstellar Jan 14, 2024
bf47583
Fix dependency install
tstellar Jan 14, 2024
c7e78a2
XXX: DEbug
tstellar Jan 14, 2024
ac6218c
write everything to a file
tstellar Jan 14, 2024
41260b5
XXX: Debug
tstellar Jan 14, 2024
5ed3b20
Fix for formatter
tstellar Jan 14, 2024
9e640ef
XXX: Fix formatter
tstellar Jan 14, 2024
f23afb5
XXX: Fix formatter
tstellar Jan 14, 2024
81ff8c1
XXX: Fix formatter
tstellar Jan 14, 2024
0b4e994
XX: fix formatter
tstellar Jan 14, 2024
04a580f
XX: fix formatter
tstellar Jan 14, 2024
99df477
XX: fix formatter
tstellar Jan 14, 2024
65c1d65
XXX: fix typo
tstellar Jan 14, 2024
1879746
XXX: fix typo
tstellar Jan 14, 2024
93c473e
Fix stray quote
tstellar Jan 14, 2024
2b093af
Revert "Break formatting"
tstellar Jan 14, 2024
bb367a8
Format fixes
tstellar Jan 14, 2024
da6c815
Revert "Revert "Break formatting""
tstellar Jan 14, 2024
6310fc2
XXX: Debug
tstellar Jan 14, 2024
2327b94
XXX: Debug
tstellar Jan 14, 2024
bb7c900
XXX: Debug
tstellar Jan 15, 2024
6610f22
XXX: Debug
tstellar Jan 15, 2024
3c4d831
XXX: Fix
tstellar Jan 15, 2024
b639702
XXX: Fix
tstellar Jan 15, 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
72 changes: 72 additions & 0 deletions .github/workflows/issue-write.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
name: Comment on an issue

on:
workflow_run:
workflows: ["Check code formatting"]
types:
- completed

permissions:
contents: read
issues: write
pull-requests: write

jobs:
upload:
runs-on: ubuntu-latest
if: >
github.event.workflow_run.event == 'pull_request'
steps:
- name: 'Download artifact'
# v7.0.1
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea
with:
script: |
let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: context.payload.workflow_run.id,
});
let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
return artifact.name == "workflow-args"
})[0];
let download = await github.rest.actions.downloadArtifact({
owner: context.repo.owner,
repo: context.repo.repo,
artifact_id: matchArtifact.id,
archive_format: 'zip',
});
let fs = require('fs');
fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/workflow-args.zip`, Buffer.from(download.data));

- run: unzip workflow-args.zip

- name: 'Comment on PR'
uses: actions/github-script@v3
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
var fs = require('fs');
const comments = JSON.parse(fs.readFileSync('./comments'));
if (!comments) {
return;
}
console.log(comments);
await comments.forEach(function (comment) {
if (comment.id) {
github.issues.updateComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: comment.number,
comment_id: comment.id,
body: comment.body
});
} else {
github.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: comment.number,
body: comment.body
});
}
});
31 changes: 12 additions & 19 deletions .github/workflows/pr-code-format.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
name: "Check code formatting"
on: pull_request_target
permissions:
pull-requests: write
on: pull_request

jobs:
code_formatter:
runs-on: ubuntu-latest
if: github.repository == 'llvm/llvm-project'
steps:
- name: Fetch LLVM sources
uses: actions/checkout@v4
Expand All @@ -27,18 +24,6 @@ jobs:
separator: ","
skip_initial_fetch: true

# We need to make sure that we aren't executing/using any code from the
# PR for security reasons as we're using pull_request_target. Checkout
# the target branch with the necessary files.
- name: Fetch code formatting utils
uses: actions/checkout@v4
with:
sparse-checkout: |
llvm/utils/git/requirements_formatting.txt
llvm/utils/git/code-format-helper.py
sparse-checkout-cone-mode: false
path: code-format-tools

- name: "Listed files"
env:
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
Expand All @@ -56,10 +41,10 @@ jobs:
with:
python-version: '3.11'
cache: 'pip'
cache-dependency-path: 'code-format-tools/llvm/utils/git/requirements_formatting.txt'
cache-dependency-path: 'llvm/utils/git/requirements_formatting.txt'

- name: Install python dependencies
run: pip install -r code-format-tools/llvm/utils/git/requirements_formatting.txt
run: pip install -r llvm/utils/git/requirements_formatting.txt

- name: Run code formatter
env:
Expand All @@ -72,9 +57,17 @@ jobs:
# explicitly in code-format-helper.py and not have to diff starting at
# the merge base.
run: |
python ./code-format-tools/llvm/utils/git/code-format-helper.py \
python ./llvm/utils/git/code-format-helper.py \
--write-comment-to-file \
--token ${{ secrets.GITHUB_TOKEN }} \
--issue-number $GITHUB_PR_NUMBER \
--start-rev $(git merge-base $START_REV $END_REV) \
--end-rev $END_REV \
--changed-files "$CHANGED_FILES"

- uses: actions/upload-artifact@v2
if: always()
with:
name: workflow-args
path: |
comments
4 changes: 1 addition & 3 deletions clang/lib/Driver/ToolChain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -428,9 +428,7 @@ ToolChain::getDefaultUnwindTableLevel(const ArgList &Args) const {
}

unsigned ToolChain::GetDefaultDwarfVersion() const {
// TODO: Remove the RISC-V special case when R_RISCV_SET_ULEB128 linker
// support becomes more widely available.
return getTriple().isRISCV() ? 4 : 5;
return 4;
}

Tool *ToolChain::getClang() const {
Expand Down
8 changes: 4 additions & 4 deletions clang/test/CodeGen/dwarf-version.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER3
// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
// RUN: %clang -target x86_64-linux-gnu -gdwarf-5 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
// RUN: %clang -target x86_64-linux-gnu -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
// RUN: %clang -target x86_64-linux-gnu -gdwarf -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
// RUN: %clang --target=i386-pc-solaris -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
// RUN: %clang --target=i386-pc-solaris -gdwarf -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
// RUN: %clang -target x86_64-linux-gnu -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
// RUN: %clang -target x86_64-linux-gnu -gdwarf -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
// RUN: %clang --target=i386-pc-solaris -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
// RUN: %clang --target=i386-pc-solaris -gdwarf -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4

// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
// environment variable which indirecty overrides the version in the target
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Driver/as-options.s
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
// RUN: FileCheck --check-prefix=DEBUG %s
// RUN: %clang --target=aarch64-linux-gnu -fno-integrated-as -g0 -g %s -### 2>&1 | \
// RUN: FileCheck --check-prefix=DEBUG %s
// DEBUG: "-g" "-gdwarf-5"
// DEBUG: "-g" "-gdwarf-4"
// RUN: %clang --target=aarch64-linux-gnu -fno-integrated-as -g -g0 %s -### 2>&1 | \
// RUN: FileCheck --check-prefix=NODEBUG %s
// RUN: %clang --target=aarch64-linux-gnu -fno-integrated-as -gdwarf-5 -g0 %s -### 2>&1 | \
Expand All @@ -141,7 +141,7 @@
// RUN: %clang --target=aarch64-linux-gnu -fno-integrated-as -gdwarf-2 %s -### 2>&1 | \
// RUN: FileCheck --check-prefix=GDWARF2 %s
// RUN: %clang --target=aarch64-linux-gnu -fno-integrated-as -gdwarf %s -### 2>&1 | \
// RUN: FileCheck --check-prefix=GDWARF5 %s
// RUN: FileCheck --check-prefix=GDWARF4 %s

// RUN: %clang --target=aarch64-linux-gnu -fno-integrated-as -gdwarf-5 %s -### 2>&1 | \
// RUN: FileCheck --check-prefix=GDWARF5 %s
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/cl-options.c
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@
// RUN: %clang_cl -gdwarf /Z7 /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s
// Z7_gdwarf-NOT: "-gcodeview"
// Z7_gdwarf: "-debug-info-kind=constructor"
// Z7_gdwarf: "-dwarf-version=
// Z7_gdwarf: "-dwarf-version=4

// RUN: %clang_cl /ZH:MD5 /c -### -- %s 2>&1 | FileCheck -check-prefix=ZH_MD5 %s
// ZH_MD5: "-gsrc-hash=md5"
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/clang-g-opts.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

// CHECK-WITHOUT-G-NOT: -debug-info-kind
// CHECK-WITH-G: "-debug-info-kind=constructor"
// CHECK-WITH-G: "-dwarf-version=5"
// CHECK-WITH-G: "-dwarf-version=4"
// CHECK-WITH-G-DWARF4: "-dwarf-version=4"
// CHECK-WITH-G-DWARF2: "-dwarf-version=2"

Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/ve-toolchain.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
/// Checking dwarf-version

// RUN: %clang -### -g --target=ve %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
// DWARF_VER: "-dwarf-version=5"
// DWARF_VER: "-dwarf-version=4"

///-----------------------------------------------------------------------------
/// Checking include-path
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/ve-toolchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

// RUN: %clangxx -### -g --target=ve-unknown-linux-gnu \
// RUN: %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
// DWARF_VER: "-dwarf-version=5"
// DWARF_VER: "-dwarf-version=4"

///-----------------------------------------------------------------------------
/// Checking include-path
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ AMDGPUTargetMachine::AMDGPUTargetMachine(const Target &T, const Triple &TT,
FS, Options, getEffectiveRelocModel(RM),
getEffectiveCodeModel(CM, CodeModel::Small), OptLevel),
TLOF(createTLOF(getTargetTriple())) {
initAsmInfo();
initAsmInfo();
if (TT.getArch() == Triple::amdgcn) {
if (getMCSubtargetInfo()->checkFeatures("+wavefrontsize64"))
MRI.reset(llvm::createGCNMCRegisterInfo(AMDGPUDwarfFlavour::Wave64));
Expand Down
33 changes: 32 additions & 1 deletion llvm/utils/git/code-format-helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class FormatArgs:
token: str = None
verbose: bool = True
issue_number: int = 0
write_comment_to_file: bool = False

def __init__(self, args: argparse.Namespace = None) -> None:
if not args is None:
Expand All @@ -53,12 +54,14 @@ def __init__(self, args: argparse.Namespace = None) -> None:
self.token = args.token
self.changed_files = args.changed_files
self.issue_number = args.issue_number
self.write_comment_to_file = args.write_comment_to_file


class FormatHelper:
COMMENT_TAG = "<!--LLVM CODE FORMAT COMMENT: {fmt}-->"
name: str
friendly_name: str
comment: dict = None

@property
def comment_tag(self) -> str:
Expand Down Expand Up @@ -110,15 +113,25 @@ def find_comment(self, pr: any) -> any:
return None

def update_pr(self, comment_text: str, args: FormatArgs, create_new: bool) -> None:

import github
from github import IssueComment, PullRequest

repo = github.Github(args.token).get_repo(args.repo)
pr = repo.get_issue(args.issue_number).as_pull_request()

comment_text = self.comment_tag + "\n\n" + comment_text

existing_comment = self.find_comment(pr)

if args.write_comment_to_file:
self.comment = {
'number' : pr.number,
'body' : comment_text
}
if existing_comment:
self.comment['id'] = existing_comment.id
return

if existing_comment:
existing_comment.edit(comment_text)
elif create_new:
Expand All @@ -128,18 +141,21 @@ def run(self, changed_files: List[str], args: FormatArgs) -> bool:
diff = self.format_run(changed_files, args)
should_update_gh = args.token is not None and args.repo is not None

print("running")
if diff is None:
if should_update_gh:
comment_text = (
":white_check_mark: With the latest revision "
f"this PR passed the {self.friendly_name}."
)
print("UPDATE with", comment_text)
self.update_pr(comment_text, args, create_new=False)
return True
elif len(diff) > 0:
if should_update_gh:
comment_text = self.pr_comment_text_for_diff(diff)
self.update_pr(comment_text, args, create_new=True)
print("UPDATE with", comment_text)
else:
print(
f"Warning: {self.friendly_name}, {self.name} detected "
Expand All @@ -153,6 +169,7 @@ def run(self, changed_files: List[str], args: FormatArgs) -> bool:
f":warning: The {self.friendly_name} failed without printing "
"a diff. Check the logs for stderr output. :warning:"
)
print("UPDATE with", comment_text)
self.update_pr(comment_text, args, create_new=False)
return False

Expand Down Expand Up @@ -309,6 +326,8 @@ def hook_main():
if fmt.has_tool():
if not fmt.run(args.changed_files, args):
failed_fmts.append(fmt.name)
if fmt.comment:
comments.append(fmt.comment)
else:
print(f"Couldn't find {fmt.name}, can't check " + fmt.friendly_name.lower())

Expand Down Expand Up @@ -349,6 +368,10 @@ def hook_main():
type=str,
help="Comma separated list of files that has been changed",
)
parser.add_argument(
"--write-comment-to-file",
action='store_true',
help="Don't create a comments on the PR, instead write the comments and metadata a file called 'comment'" )

args = FormatArgs(parser.parse_args())

Expand All @@ -357,9 +380,17 @@ def hook_main():
changed_files = args.changed_files.split(",")

failed_formatters = []
comments = []
for fmt in ALL_FORMATTERS:
if not fmt.run(changed_files, args):
failed_formatters.append(fmt.name)
if fmt.comment:
comments.append(fmt.comment)

if len(comments):
with open('comments', 'w') as f:
import json
json.dump(comments, f)

if len(failed_formatters) > 0:
print(f"error: some formatters failed: {' '.join(failed_formatters)}")
Expand Down
Loading