action: add license check #85
Conversation
.github/workflows/license.yml
Outdated
| run: | | ||
| import os, datetime, sys | ||
|
|
||
| intel_license: list[str] = [ |
There was a problem hiding this comment.
shall we put this script in .github or somewhere inside our repo?
There was a problem hiding this comment.
@Menooker
If developers need to run it as a local check, it is ok to split the script into a seperate python file.
Do you really need to do the local check?
There was a problem hiding this comment.
Please also note that we already have script/ directory for compiling the repo and compiling LLVM. The sh files are used in CI too. I think we should add the py file to this directory.
There was a problem hiding this comment.
How about put this check script into repo_dir/scripts/license_check.py, so that this script can be used not only in actions but also from local.
For complex logic, normally put them into scrips/
.github/workflows/license.yml
Outdated
| ] | ||
|
|
||
| llvm_license: list[str] = [ | ||
| 'This file is licensed under the Apache License v2.0 with LLVM Exceptions.', |
There was a problem hiding this comment.
LLVM's header looks like
//===- Example.cpp - Tests for Example ------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
Shall we check the header //===- Example.cpp - Tests for Example ------------------------------------===// and the footer //===----------------------------------------------------------------------===// as well?
Note that the header has variable number of ------------------------------------ to align the line wrap limit.
There was a problem hiding this comment.
@Menooker
The string Tests for Example you mentioned is not a costant string or from the filename. I do not know how to verify it.
That's why I skipped the first line check for llvm license.
There was a problem hiding this comment.
It is {file name} - {desc} ---------. I think we can check the filename and the number of ---------. For auto-fix, we can fill-in the filename and a place holder "{DESC}" and auto fix the number of ------------.
In checking mode, we can throw an error when detecting "{DESC}".
A workflow for the user to use auto-fix should be:
- write code without LLVM header
- auto-fix for the first time
- fill in {DESC} in the code
- auto-fix again, maybe check if there are remaining {DESC}
.github/workflows/license.yml
Outdated
| 'SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception', | ||
| ] | ||
|
|
||
| def check_license(filename: str, license: list[str]): |
There was a problem hiding this comment.
oneDNN's script also provides a utility to auto fix the headers. It is useful when a PR is across two years (to update year).
I think for LLVM, auto fixing headers are also useful, when we introduce many cpp files in a PR, and we need to manually fill-in the file names in the licence. Maybe we can improve the script in another PR.
There was a problem hiding this comment.
We can write another small script to handle the fixing job in the future, but not in the automatic CI script.
WangJialei-A
force-pushed
the
wangjial/check_license
branch
from
e519fef
to
0adefdb
Compare
scripts/license.py
Outdated
| year: int = datetime.datetime.now().year | ||
| success: bool = True | ||
|
|
||
| for filepath in os.environ["CHANGED_FILES"].split(','): |
There was a problem hiding this comment.
shall we pass this by args, so that users can check it locally?
test/lit.cfg.py
Outdated
| @@ -1,3 +1,19 @@ | |||
| # Copyright (C) 2024 Intel Corporation | |||
| # | |||
There was a problem hiding this comment.
It is part of MLIR testing. And upstream MLIR does not add copyright on this file
There was a problem hiding this comment.
It is part of MLIR testing. And upstream MLIR does not add copyright on this file
@Menooker It is Intel license, not the LLVM one and non-related with upstream work.
There was a problem hiding this comment.
It is copied and modified based on upstream MLIR. In upstream, this file seems not to have copyright?
scripts/license.py
Outdated
| success = False | ||
| print("Fail : %s" % filepath) | ||
| else: | ||
| print("Success: %s" % filepath) |
There was a problem hiding this comment.
Can we provide suggestions on how to fix that?
WangJialei-A
force-pushed
the
wangjial/check_license
branch
2 times, most recently
from
3f08c68
to
6a1274f
Compare
WangJialei-A
force-pushed
the
wangjial/check_license
branch
from
6a1274f
to
01f4824
Compare
WangJialei-A
force-pushed
the
wangjial/check_license
branch
from
01f4824
to
72abd7a
Compare
Add license check
Files in the changeset with suffix
.c,.cpp,.h,.hpp,.py,CMakeLists.txtwill be checked.Please see the result at #86 and url