Skip to content

Commit

Permalink
Add license check and formatting check (#156)
Browse files Browse the repository at this point in the history
Add docs/Contributing.md
Add .editorconfig
Apply clang formatting to existing files

Signed-off-by: Alan Jowett <alanjo@microsoft.com>

Signed-off-by: Alan Jowett <alanjo@microsoft.com>
  • Loading branch information
Alan-Jowett committed Oct 20, 2022
1 parent 049f603 commit 5cd99a6
Show file tree
Hide file tree
Showing 33 changed files with 1,813 additions and 639 deletions.
23 changes: 23 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
BasedOnStyle: LLVM
IndentWidth: 4
ColumnLimit: 120
AlignEscapedNewlines: Left
AlignAfterOpenBracket: AlwaysBreak
#
# Bind * to the type rather than the name.
PointerAlignment: Left
#
# Put function name on separate line from return type.
AlwaysBreakAfterReturnType: All
#
# Put arguments either all on same line or on separate lines.
BinPackArguments: false
#
# Put function parameters on separate lines.
BinPackParameters: false
#
# Open brace goes on new line only when starting a new struct, enum, or func.
BreakBeforeBraces: Mozilla
#
# Don't sort includes in alphabetical order because Windows headers are odd.
SortIncludes: false
12 changes: 12 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright (c) 2022-present, IO Visor Project
# SPDX-License-Identifier: Apache-2.0

# top-most EditorConfig file
root = true

[*]
indent_style = space
indent_size = 4
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,5 @@ dmypy.json
# Pyre type checker
.pyre/

.vscode/
.vscode/
.vs/
9 changes: 8 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#
# Copyright (c) 2022-present, IO Visor Project
# SPDX-License-Identifier: Apache-2.0
#
# All rights reserved.
#
# This source code is licensed in accordance with the terms specified in
Expand All @@ -9,6 +10,12 @@
project("ubpf")
cmake_minimum_required(VERSION 3.16)

if (UBPF_INSTALL_GIT_HOOKS)
# Install Git pre-commit hook
file(COPY scripts/pre-commit scripts/commit-msg
DESTINATION "${PROJECT_SOURCE_DIR}/.git/hooks")
endif()

include("cmake/platform.cmake")
include("cmake/settings.cmake")
include("cmake/options.cmake")
Expand Down
2 changes: 1 addition & 1 deletion aarch64_test/run-interpret.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash
# Copyright (c) Microsoft Corporation
# SPDX-License-Identifier: MIT
# SPDX-License-Identifier: Apache-2.0

# Work around for argument passing.
qemu-aarch64 -L /usr/aarch64-linux-gnu build/bin/ubpf_plugin "$*" --interpret
2 changes: 1 addition & 1 deletion aarch64_test/run-jit.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash
# Copyright (c) Microsoft Corporation
# SPDX-License-Identifier: MIT
# SPDX-License-Identifier: Apache-2.0

# Work around for argument passing.
qemu-aarch64 -L /usr/aarch64-linux-gnu build/bin/ubpf_plugin "$*" --jit
3 changes: 2 additions & 1 deletion cmake/options.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2022-present, IO Visor Project
# SPDX-License-Identifier: Apache-2.0
# All rights reserved.
#
# This source code is licensed in accordance with the terms specified in
Expand All @@ -15,6 +15,7 @@ option(UBPF_ENABLE_INSTALL "Set to true to enable the install targets")
option(UBPF_ENABLE_TESTS "Set to true to enable tests")
option(UBPF_ENABLE_PACKAGE "Set to true to enable packaging")
option(UBPF_SKIP_EXTERNAL "Set to true to skip external projects")
option(UBPF_INSTALL_GIT_HOOKS "Set to true to install git hooks" ON)

# Note that the compile_commands.json file is only exporter when
# using the Ninja or Makefile generator
Expand Down
127 changes: 127 additions & 0 deletions docs/Contributing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
Development Guide
=================

Coding Conventions
------------------

* **DO** use fixed length types defined in `stdint.h` instead
of language keywords determined by the compiler (e.g., `int64_t, uint8_t`, not
`long, unsigned char`).

* **DO** use `const` and `static` and visibility modifiers to scope exposure of
variables and methods as much as possible.

* **DO** use doxygen comments, with \[in,out\]
[direction annotation](http://www.doxygen.nl/manual/commands.html#cmdparam) in all public API
headers. This is also encouraged, but not strictly required, for internal API
headers as well.

* **DON'T** use global variables where possible.

* **DON'T** use abbreviations unless they are already well-known terms known by
users (e.g., "app", "info"), or are already required for use by developers (e.g.,
"min", "max", "args"). Examples of bad use would be `num_widgets` instead of
`widget_count`, and `opt_widgets` instead of `option_widgets` or `optional_widgets`.

* **DON'T** use hard-coded magic numbers for things that have to be consistent
between different files. Instead use a `#define` or an enum or const value, as appropriate.

* **DON'T** use the same C function name with two different prototypes across
the project where possible.

* **DON'T** use commented-out code, or code in an `#if 0` or equivalent. Make sure all code is actually
built.

Header Files
------------

* **DO** make sure any header file can be included directly, without requiring other
headers to be included first. That is, any dependencies should be included within
the header file itself.

* **DO** include system headers (with `<>`) before local headers (with `""`), and list them
in alphabetical order where possible. This helps ensure there are not duplicate includes,
and also helps ensure that headers are usable directly.

* **DO** use `#pragma once` in all header files, rather than using ifdefs to test for duplicate inclusion.

Style Guide
-----------

### Automated Formatting with `clang-format`

For all C/C++ files (`*.c`, `*.cpp` and `*.h`), we use `clang-format` (specifically
version 3.6) to apply our code formatting rules. After modifying C/C++ files and
before merging, be sure to run:

```sh
$ ./scripts/format-code
```

This allows us to apply formatting choices such as the use of [Allman style](
http://en.wikipedia.org/wiki/Indent_style#Allman_style) braces and the 80
character column width consistently.

Please stage the formatting changes with your commit, instead of making an extra
"Format Code" commit. Your editor can likely be set up to automatically run
`clang-format` across the file or region you're editing. See:

- [clang-format.el](https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/clang-format.el) for Emacs
- [vim-clang-format](https://github.com/rhysd/vim-clang-format) for Vim
- [vscode-cpptools](https://marketplace.visualstudio.com/items?itemName=ms-vscode.cpptools)
for Visual Studio Code

The [.clang-format](../.clang-format) file describes the style that is enforced
by the script, which is based off the LLVM style with modifications closer to
the default Visual Studio style. See [clang-format style options](
http://releases.llvm.org/3.6.0/tools/clang/docs/ClangFormatStyleOptions.html)
for details.

If you see unexpected formatting changes in the code, verify that you are running version 11 or higher of the LLVM tool-chain.

### License Header

The following license header **must** be included at the top of every new code file:

```
// Copyright (c) <Contributor>
// SPDX-License-Identifier: Apache-2.0
```

It should be prefixed with the file's comment marker. If there is a compelling
reason to not include this header, the file can be added to
`.check-license.ignore`.

All files are checked for this header with the script:

```sh
$ ./scripts/check-license
```

### Naming Conventions

Naming conventions we use that are not automated include:

1. Use `lower_snake_case` for variable, member/field, and function names.
2. Use `UPPER_SNAKE_CASE` for macro names and constants.
3. Prefer `lower_snake_case` file names for headers and sources.
4. Prefer full words for names over contractions (i.e., `memory_context`, not
`mem_ctx`).
5. Prefix names with `_` to indicate internal and private fields or methods
(e.g., `_internal_field, _internal_method()`).
6. The single underscore (`_` ) is reserved for local definitions (static,
file-scope definitions).
e.g., static ubpf_result_t _do_something(..).
7. Prefix `struct` definitions with `_` (this is an exception to point 6), and always create a `typedef` with the
suffix `_t`. For example:
```c
typedef struct _ubpf_widget
{
uint64_t count;
} ubpf_widget_t;
```
8. Prefix uBPF specific names in the global namespace with `ubpf_` (e.g., `ubpf_result_t`).

Above all, if a file happens to differ in style from these guidelines (e.g.,
private members are named `m_member` rather than `_member`), the existing style
in that file takes precedence.
25 changes: 25 additions & 0 deletions scripts/.check-license.ignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# This file uses regular expressions, not shell globs!

# File extensions that don't support embedding license info
.*\.md$
.*\.o$
.*\.png$
.*\.proj$
.*\.rc$
.*\.sln$
.*\.vsdx$

# Generated files
tools/netsh/resource.h

# Other Files
LICENSE.txt
Doxyfile
\.check-license\.ignore
\.clang-format
\.gitattributes
\.github/CODEOWNERS
\.github/workflows/build.yml
\.gitignore
\.gitmodules
packages.config
86 changes: 86 additions & 0 deletions scripts/check-license.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#!/usr/bin/env bash

# Copyright (c) Microsoft Corporation
# SPDX-License-Identifier: Apache-2.0

# This script accepts either a list of files relative to the current
# working directory, or it will check all files tracked in Git. In
# both cases, it ignores files matching any regular expression listed
# in '.check-license.ignore'.

set -o errexit
set -o pipefail

license=("Copyright" "SPDX-License-Identifier:")

root=$(git rev-parse --show-toplevel)

# If we are inside mingw* environment, then we update the path to proper format
if [[ $(uname) == MINGW* ]] ; then
root=$(cygpath -u "${root}")
fi

ignore_res=()
while IFS=$'\r' read -r i; do
if [[ $i =~ ^# ]] || [[ -z $i ]]; then # ignore comments
continue
fi
ignore_res+=("$i")
done < "$root/scripts/.check-license.ignore"

should_ignore() {
for re in "${ignore_res[@]}"; do
if [[ $1 =~ $re ]]; then
return
fi
done
false
}

# Create array of files to check, either from the given arguments or
# all files in Git, ignore any that match a regex in the ignore file.
files=()
if [[ $# -ne 0 ]]; then
for f in "$@"; do
file=$(realpath -q "$f") || file=""

if [[ ! -f $file ]]; then # skip non-existent files
continue
fi

file=${file#$root/} # remove the prefix

if should_ignore "$file"; then
continue
fi

files+=("$file")
done
else
# Find all files in Git. These are guaranteed to exist, to not be
# generated, and to not have the prefix.
cd "$root"
while IFS= read -r -d '' file; do
if should_ignore "$file"; then
continue
fi

files+=("$file")
done < <(git ls-files -z)
fi

failures=0
for file in "${files[@]}"; do
for line in "${license[@]}"; do
# We check only the first four lines to avoid false positives
# (such as this script), but to allow for a shebang and empty
# line between it and the license.
if ! head -n4 "${root}/${file}" | grep --quiet --fixed-strings --max-count=1 "${line}"; then
echo "${file}" missing "${line}"
failures=$((failures + 1))
break
fi
done
done

exit $failures
25 changes: 25 additions & 0 deletions scripts/commit-msg
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/usr/bin/env bash

# Copyright (c) Microsoft Corporation
# SPDX-License-Identifier: Apache-2.0

set -o errexit

exit_() {
echo ""
echo "$1"
echo ""
echo "This hook can be skipped if needed with 'git commit --no-verify'"
echo "See '.git/hooks/commit-msg., installed from 'scripts/commit-msg'"
exit 1
}

sign_offs="$(grep '^Signed-off-by: ' "$1" || test $? = 1 )"

if [[ -z $sign_offs ]]; then
exit_ "Commit failed: please sign-off on the DCO with 'git commit -s'"
fi

if [[ -n $(echo "$sign_offs" | sort | uniq -c | sed -e '/^[ ]*1[ ]/d') ]]; then
exit_ "Commit failed: please remove duplicate Signed-off-by lines"
fi
Loading

0 comments on commit 5cd99a6

Please sign in to comment.