From 50550708ccc069ed127197761d400892e8dcde02 Mon Sep 17 00:00:00 2001 From: Gerald Combs Date: Tue, 18 Aug 2020 16:00:32 -0700 Subject: [PATCH] Add merge request jobs to GitLab CI and migrate commit validation. Copy the Buildbot petri dish builder steps to corresponding GitLab CI jobs. Update validate-commit.py to look for old "Bug:" and "Ping-Bug:" references and have it call `git stripspace` directly. tools/commit-msg was specific to Gerrit, so remove it. Change-Id: Icbc54709052f44c941db9ad6a5dcf596292782a2 --- .editorconfig | 5 + .gitlab-ci.yml | 94 ++++++++++++- tools/commit-msg | 282 --------------------------------------- tools/pre-commit | 2 +- tools/validate-commit.py | 36 +++-- 5 files changed, 114 insertions(+), 305 deletions(-) delete mode 100755 tools/commit-msg diff --git a/.editorconfig b/.editorconfig index 1bd0e713cd..cb6c225d87 100644 --- a/.editorconfig +++ b/.editorconfig @@ -39,6 +39,11 @@ indent_size = 2 indent_style = space indent_size = 2 +# YAML +[*.{yml}] +indent_style = space +indent_size = 2 + # C/C++ [*.{c,cpp,h}] indent_style = space diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index da483d47f4..7025afc5a1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -8,6 +8,8 @@ # The custom Ubuntu image pre-installs dependencies and compilers to speed up the build: # https://hub.docker.com/r/wireshark/wireshark-ubuntu-dev # https://github.com/wireshark/wireshark-ubuntu-dev-docker +# XXX - We might be able to speed things up using ccache: +# https://gould.cx/ted/blog/2017/06/10/ccache-for-Gitlab-CI/ .build-ubuntu: &build-ubuntu <<: *build image: wireshark/wireshark-ubuntu-dev @@ -37,7 +39,7 @@ expire_in: 3 days # Rely on fedora:latest and debian-stable jobs for testing a recent GCC version. -clang-10: +clang-10: &clang-10 <<: *build-ubuntu variables: CC: clang-10 @@ -133,3 +135,93 @@ test:debian-stable: GIT_STRATEGY: none dependencies: - build:debian-stable + +# https://docs.gitlab.com/ee/user/gitlab_com/index.html#linux-shared-runners +merge-request:ubuntu-dpkg: + <<: *build-ubuntu + tags: + - docker + only: + - merge_requests + script: + - apt-get install -y devscripts + # build-ubuntu puts us in `build`. + - cd .. + # From the Buildbot Petri Dish. We might want to spread these across different builders. + - bash ./tools/pre-commit 'HEAD^1' + - sh -c '[ ! -e tools/validate-commit.py ] || tools/validate-commit.py' + - dpkg-buildpackage -us -uc -rfakeroot -jauto -Zgzip -zfast + - lintian --suppress-tags library-not-linked-against-libc --display-experimental --display-info --pedantic --profile debian + +merge-request:ubuntu-gcc-ctest: + <<: *build-ubuntu + tags: + - docker + only: + - merge_requests + script: + # build-ubuntu puts us in `build`. + - perl ../tools/make-version.pl --set-release || ../perl make-version.pl --set-release + - CC=gcc CXX=g++ cmake -DENABLE_EXTRA_COMPILER_WARNINGS=on -DCMAKE_EXPORT_COMPILE_COMMANDS=on -G Ninja .. + - ninja + - ninja test-programs + - chown -R user . + - su user -c "ctest --parallel 3 --force-new-ctest-process --verbose" + +merge-request:ubuntu-clang-other-tests: + extends: clang-10 + tags: + - docker + only: + - merge_requests + script: + - apt-get install -y cppcheck + # build-ubuntu puts us in `build`. + - cd .. + - python3 tools/checklicenses.py + - ./tools/cppcheck/cppcheck.sh -l 1 + - cd build + - cmake -DENABLE_EXTRA_COMPILER_WARNINGS=on -DENABLE_CHECKHF_CONFLICT=on -DCMAKE_EXPORT_COMPILE_COMMANDS=on -G Ninja .. + - ninja + - ./run/tshark -v + - sh -c '[ ! -e ../tools/validate-clang-check.py ] || ../tools/validate-clang-check.py' + - ninja checkAPI + +# https://docs.gitlab.com/ee/user/gitlab_com/index.html#windows-shared-runners-beta +# Disabled for now due to +# https://gitlab.com/gitlab-org/gitlab-runner/-/issues/26788 +.merge-request:windows: + tags: + - wireshark-windows-dev + stage: build + only: + - merge_requests + before_script: + # XXX Find a better location. + - mkdir c:\Development + - $env:WIRESHARK_BASE_DIR = "C:\Development" + - $env:Configuration = "RelWithDebInfo" + # This takes a while. Should we create our own Docker image? + #- choco install -y winflexbison3 cmake strawberryperl python3 + # asciidoctorj xsltproc docbook-bundle? + - $env:Path += ";C:\Program Files\CMake\bin" + - $env:Path += ";C:\Strawberry\c\bin;C:\Strawberry\perl\site\bin;C:\Strawberry\perl\bin" + - $env:Path += ";C:\qt\5.12.8\msvc2017_64\bin" + # https://help.appveyor.com/discussions/questions/18777-how-to-use-vcvars64bat-from-powershell + - cmd.exe /c "call `"C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Auxiliary\Build\vcvars64.bat`" && set > %temp%\vcvars.txt" + - Get-Content "$env:temp\vcvars.txt" | Foreach-Object { if ($_ -match "^(.*?)=(.*)$") { Set-Content "env:\$($matches[1])" $matches[2] } } + # Testing / debugging only. + - dir c:\ + - dir c:\qt + - $env:path.split(";") + - cmd.exe /c "set" + - Get-Location + script: + # From the Buildbot Petri Dish. We might want to spread these across different bulders. + - perl tools/make-version.pl --set-release + - mkdir build + - cd build + - cmake -G "Visual Studio 16 2019" -A x64 .. + - msbuild "/consoleloggerparameters:PerformanceSummary;NoSummary" /maxcpucount Wireshark.sln + - msbuild "/consoleloggerparameters:PerformanceSummary;NoSummary" test-programs.vcxproj + - ctest -C RelWithDebInfo --parallel 3 --force-new-ctest-process --verbose diff --git a/tools/commit-msg b/tools/commit-msg deleted file mode 100755 index 0dab2d7687..0000000000 --- a/tools/commit-msg +++ /dev/null @@ -1,282 +0,0 @@ -#!/bin/sh -# From Gerrit Code Review 2.12.7 -# -# Part of Gerrit Code Review (https://www.gerritcodereview.com/) -# -# Copyright (C) 2009 The Android Open Source Project -# -# SPDX-License-Identifier: Apache-2.0 -# - -unset GREP_OPTIONS - -CHANGE_ID_AFTER="Bug|Issue|Ping-Bug|Test" -MSG="$1" - -# Check for, and add if missing, a unique Change-Id -# -add_ChangeId() { - clean_message=`sed -e ' - /^diff --git .*/{ - s/// - q - } - /^Signed-off-by:/d - /^#/d - ' "$MSG" | git stripspace` - if test -z "$clean_message" - then - return - fi - - # Do not add Change-Id to temp commits - if echo "$clean_message" | head -1 | grep -q '^\(fixup\|squash\)!' - then - return - fi - - if test "false" = "`git config --bool --get gerrit.createChangeId`" - then - return - fi - - # Do not add Change-Id on packaging branches - if git branch | grep -q '^\* \(debian\|ubuntu\)/' - then - return - fi - - # Does Change-Id: already exist? if so, exit (no change). - if grep -i '^Change-Id:' "$MSG" >/dev/null - then - return - fi - - id=`_gen_ChangeId` - T="$MSG.tmp.$$" - AWK=awk - if [ -x /usr/xpg4/bin/awk ]; then - # Solaris AWK is just too broken - AWK=/usr/xpg4/bin/awk - fi - - # Get core.commentChar from git config or use default symbol - commentChar=`git config --get core.commentChar` - commentChar=${commentChar:-#} - - # How this works: - # - parse the commit message as (textLine+ blankLine*)* - # - assume textLine+ to be a footer until proven otherwise - # - exception: the first block is not footer (as it is the title) - # - read textLine+ into a variable - # - then count blankLines - # - once the next textLine appears, print textLine+ blankLine* as these - # aren't footer - # - in END, the last textLine+ block is available for footer parsing - $AWK ' - BEGIN { - # while we start with the assumption that textLine+ - # is a footer, the first block is not. - isFooter = 0 - footerComment = 0 - blankLines = 0 - } - - # Skip lines starting with commentChar without any spaces before it. - /^'"$commentChar"'/ { next } - - # Skip the line starting with the diff command and everything after it, - # up to the end of the file, assuming it is only patch data. - # If more than one line before the diff was empty, strip all but one. - /^diff --git / { - blankLines = 0 - while (getline) { } - next - } - - # Count blank lines outside footer comments - /^$/ && (footerComment == 0) { - blankLines++ - next - } - - # Catch footer comment - /^\[[a-zA-Z0-9-]+:/ && (isFooter == 1) { - footerComment = 1 - } - - /]$/ && (footerComment == 1) { - footerComment = 2 - } - - # We have a non-blank line after blank lines. Handle this. - (blankLines > 0) { - print lines - for (i = 0; i < blankLines; i++) { - print "" - } - - lines = "" - blankLines = 0 - isFooter = 1 - footerComment = 0 - } - - # Detect that the current block is not the footer - (footerComment == 0) && (!/^\[?[a-zA-Z0-9-]+:/ || /^[a-zA-Z0-9-]+:\/\//) { - isFooter = 0 - } - - { - # We need this information about the current last comment line - if (footerComment == 2) { - footerComment = 0 - } - if (lines != "") { - lines = lines "\n"; - } - lines = lines $0 - } - - # Footer handling: - # If the last block is considered a footer, splice in the Change-Id at the - # right place. - # Look for the right place to inject Change-Id by considering - # CHANGE_ID_AFTER. Keys listed in it (case insensitive) come first, - # then Change-Id, then everything else (eg. Signed-off-by:). - # - # Otherwise just print the last block, a new line and the Change-Id as a - # block of its own. - END { - unprinted = 1 - if (isFooter == 0) { - print lines "\n" - lines = "" - } - changeIdAfter = "^(" tolower("'"$CHANGE_ID_AFTER"'") "):" - numlines = split(lines, footer, "\n") - for (line = 1; line <= numlines; line++) { - if (unprinted && match(tolower(footer[line]), changeIdAfter) != 1) { - unprinted = 0 - print "Change-Id: I'"$id"'" - } - print footer[line] - } - if (unprinted) { - print "Change-Id: I'"$id"'" - } - }' "$MSG" > "$T" && mv "$T" "$MSG" || rm -f "$T" -} -_gen_ChangeIdInput() { - echo "tree `git write-tree`" - if parent=`git rev-parse "HEAD^0" 2>/dev/null` - then - echo "parent $parent" - fi - echo "author `git var GIT_AUTHOR_IDENT`" - echo "committer `git var GIT_COMMITTER_IDENT`" - echo - printf '%s' "$clean_message" -} -_gen_ChangeId() { - _gen_ChangeIdInput | - git hash-object -t commit --stdin -} - -reject_long_message() { - read -r firstline < "$MSG" - if [ ${#firstline} -gt 80 ]; then - printf "\n\nERROR: First line in the commit message must not exceed 80 characters\n\n" - # Allow slightly longer lines in case commits are reverted. - if [ ${#firstline} -gt 100 ]; then - exit 1 - fi - fi -} - -# Remove trailing spaces, fix blank lines, fix capitalization of "Bug" tags. -fixup_message() { - # Remove trailing tabs and spaces - tab=$(printf '\t') - msg="$(sed -e "s/[${tab} ]*$//" "$MSG")" - - # Ensures proper positioning of "Ping-Bug" or "Bug" tags: - # - Must be preceded by one blank line. - # - Must not be succeeded by a blank line. - # - Must not mistake "bug:" in the description for tags in contexts such - # as "to reproduce this\nbug: 1) step 2) another step 3) etc.". - # - If preceded by other tags (like Change-Id), do not add newline. - msg=$(printf "%s\n" "$msg" | awk ' - # Eat the "--verbose" diff - /^#.*(8<|>8)/ { - while (getline) { } - next - } - # Ignore other comments - /^#/ { - next - } - - # Find "Bug" lines that potentially need to be fixed up. Assume that - # relevant bug numbers are at least four digits. - { - if (match(tolower($0), /^(ping-)?bug: *[0-9][0-9][0-9][0-9]/)) { - if (buffer == "") { - needsnewline = !lastblank && !potentialTag; - buffer = $0 - } else { - buffer = buffer "\n" $0 - } - next - } - } - - # Buffer tags and other blank lines when a fixup is potentially needed. - { potentialTag = 0 } - /^[a-zA-Z0-9-]+:/ || /^$/ { - potentialTag = 1 - if (buffer != "") { - buffer = buffer "\n" $0; - next - } - } - - # If some other text was found, assume it was not part of the footer. - # Flush any buffer, unmodified. - { - if (buffer != "") { - print buffer; - buffer = "" - } - print; - # Remember whether the last line was blank. - lastblank = $0 == "" - } - - END { - # If any tags block is still open, fix up and print it. - if (buffer != "") { - if (needsnewline) { - print "" - } - numlines = split(buffer, lines, "\n") - for (line = 1; line <= numlines; line++) { - s = lines[line]; - if (s != "") { - # Fix capitalization - sub(/^[Bb][Uu][Gg]/, "Bug", s); - sub(/^[Pp][Ii][Nn][Gg]-[Bb][Uu][Gg]/, "Ping-Bug", s); - # ensure single space after colon - sub(/: */, ": ", s); - print s; - } - } - } - } - ') && printf "%s\n" "$msg" > "$MSG" -} - - -add_ChangeId -reject_long_message -fixup_message diff --git a/tools/pre-commit b/tools/pre-commit index 636fe50caa..50c5230be0 100755 --- a/tools/pre-commit +++ b/tools/pre-commit @@ -29,7 +29,7 @@ case "$UNAME" in pyvar="pythonw.exe" ;; *) - pyvar="python" + pyvar="python3" ;; esac diff --git a/tools/validate-commit.py b/tools/validate-commit.py index 64d3e77893..417ae37688 100755 --- a/tools/validate-commit.py +++ b/tools/validate-commit.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # Verifies whether commit messages adhere to the standards. # Checks the author name and email and invokes the tools/commit-msg script. # Copy this into .git/hooks/post-commit @@ -130,27 +130,21 @@ Please rewrite your commit message to our standards, matching this format: Use paragraphs to improve readability. Limit each line to 80 characters. ''') - fd, filename = tempfile.mkstemp() - try: - os.close(fd) - with open(filename, 'w') as f: - f.write(body) - - hook_script = os.path.join(tools_dir(), 'commit-msg') - cmd = ['sh', hook_script, filename] - subprocess.check_output(cmd, universal_newlines=True) - - with open(filename, 'r') as f: - newbody = f.read() - except OSError as ex: - print('Warning: unable to invoke commit-msg hook: %s' % (ex,)) - return is_good - except subprocess.CalledProcessError as ex: - print('Bad commit message (reported by tools/commit-msg):') - print(ex.output.strip()) + if any(line.startswith('Bug:') or line.startswith('Ping-Bug:') for line in old_lines): + sys.stderr.write(''' +To close an issue, use "Closes #1234" or "Fixes #1234" instead of "Bug: 1234". +To reference an issue, use "related to #1234" instead of "Ping-Bug: 1234". See +https://docs.gitlab.com/ee/user/project/issues/managing_issues.html#closing-issues-automatically +for details. +''') return False - finally: - os.unlink(filename) + + try: + cmd = ['git', 'stripspace'] + newbody = subprocess.check_output(cmd, input=body, universal_newlines=True) + except OSError as ex: + print('Warning: unable to invoke git stripspace: %s' % (ex,)) + return is_good if newbody != body: new_lines = newbody.splitlines(True) diff = difflib.unified_diff(old_lines, new_lines,