Attention is currently required from: Felix Singer.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70243 )
Change subject: util/lint: Introduce linter for git sign-off-by line
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/70243
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.3.x
Gerrit-Change-Id: I03f1827803f8492d60a0a44174d5822c2265bfcb
Gerrit-Change-Number: 70243
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Comment-Date: Fri, 02 Dec 2022 06:04:58 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Anastasia Klimchuk.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70204 )
Change subject: flashchips.c: Add 4BA write to XM25Qx256C
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/70204
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.3.x
Gerrit-Change-Id: I96c80762fcda2af6028c7a53d8c545b0c6565cbd
Gerrit-Change-Number: 70204
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Liam Flaherty <liamflaherty(a)chromium.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Fri, 02 Dec 2022 06:04:49 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Alexander Goncharov.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68247 )
Change subject: util: add bash completion script
......................................................................
Patch Set 9: Code-Review+1
(5 comments)
Patchset:
PS9:
Just some minor issues. Otherwise looks good to me.
File Makefile:
https://review.coreboot.org/c/flashrom/+/68247/comment/548f9631_ce2d77a1
PS9, Line 589: ACTIVE_PROGRAMMERS += internal
It took a little until I understood this programmer list is needed for the substitution. Please add a short note in the commit message mentioning that this is added in the Makefile and its purpose.
https://review.coreboot.org/c/flashrom/+/68247/comment/e720674b_819ef42a
PS9, Line 1032: $(PROGRAM).8 $(PROGRAM).8.html $(PROGRAM).bash $(BUILD_DETAILS_FILE)
One tab too much
https://review.coreboot.org/c/flashrom/+/68247/comment/e391935a_689a6738
PS9, Line 1041: .bash
> IIRC there should be no `. […]
That's just the file in the source directory. I think we can't remove the file suffix that easily, else it gets in conflict with the flashrom binary since both have the same same. Though I agree the installed file shouldn't have an suffix anymore.
Actually, I like that the resulting file has a suffix. I suggest using `.bash-completion` for that file lying in the source directory. Then it's consistent with the template file (not two different suffixes). Just remove the suffix for the destination in the install command.
File util/flashrom.bash-completion.tmpl:
https://review.coreboot.org/c/flashrom/+/68247/comment/74b2f75f_06fe7c64
PS9, Line 43: OPTS="--help
Missing \ at the end of each line.
--
To view, visit https://review.coreboot.org/c/flashrom/+/68247
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie68bc91c3cea4de2ffdbeffd07e48edd8d5590e1
Gerrit-Change-Number: 68247
Gerrit-PatchSet: 9
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Fri, 02 Dec 2022 05:35:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/70247
to look at the new patch set (#3).
Change subject: cbtables.c/search_lb_records: Drop unused variable `count`
......................................................................
cbtables.c/search_lb_records: Drop unused variable `count`
Clang 15 complains about it. Remove it.
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Change-Id: I340208f513bed57a9cc2bba880a2400352c5cc42
---
M cbtable.c
1 file changed, 12 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/70247/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/70247
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I340208f513bed57a9cc2bba880a2400352c5cc42
Gerrit-Change-Number: 70247
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70247 )
Change subject: cbtables.c/search_lb_records: Drop unused variable `count`
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Sorry for the mess. For whatever reason the other review is removed when I add a new one.
--
To view, visit https://review.coreboot.org/c/flashrom/+/70247
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I340208f513bed57a9cc2bba880a2400352c5cc42
Gerrit-Change-Number: 70247
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 02 Dec 2022 02:23:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Felix Singer has uploaded a new patch set (#2). ( https://review.coreboot.org/c/flashrom/+/70247 )
Change subject: cbtables.c/search_lb_records: Drop unused variable `count`
......................................................................
cbtables.c/search_lb_records: Drop unused variable `count`
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Change-Id: I340208f513bed57a9cc2bba880a2400352c5cc42
---
M cbtable.c
1 file changed, 10 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/70247/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/70247
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I340208f513bed57a9cc2bba880a2400352c5cc42
Gerrit-Change-Number: 70247
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: newpatchset
Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70243 )
Change subject: util/lint: Introduce linter for git sign-off-by line
......................................................................
util/lint: Introduce linter for git sign-off-by line
For compliance and to check if the commit message matches the
development guidelines, add a linter checking for the sign-off-by line
in the commit message. Also, hook it up to the test_build.sh script
which is used for CI builds.
These scripts were copied from the coreboot repository and were adjusted
so that they work for flashrom.
Original-Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/70079
Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Original-Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
Change-Id: I03f1827803f8492d60a0a44174d5822c2265bfcb
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
---
M test_build.sh
A util/lint/helper_functions.sh
A util/lint/lint-extended-020-signed-off-by
3 files changed, 98 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/70243/1
diff --git a/test_build.sh b/test_build.sh
index 69d3a34..813d0b9 100755
--- a/test_build.sh
+++ b/test_build.sh
@@ -26,6 +26,11 @@
fi
+run_linter() {
+ ./util/lint/lint-extended-020-signed-off-by
+}
+
+
build_make () {
make clean
make -j $(nproc) CONFIG_EVERYTHING=yes
@@ -67,5 +72,7 @@
}
+run_linter
+
build_make
build_meson
diff --git a/util/lint/helper_functions.sh b/util/lint/helper_functions.sh
new file mode 100644
index 0000000..7abdab8
--- /dev/null
+++ b/util/lint/helper_functions.sh
@@ -0,0 +1,45 @@
+#!/usr/bin/env sh
+#
+# SPDX-License-Identifier: GPL-2.0-only
+
+# This file is sourced by the linters so that each one doesn't have to
+# specify these routines individually
+
+LC_ALL=C export LC_ALL
+
+if [ -z "$GIT" ]; then
+ GIT="$(command -v git)"
+else
+ # If git is specified, Do a basic check that it runs and seems like
+ # it's actually git
+ if ! "${GIT}" --version | grep -q git; then
+ echo "Error: ${GIT} does not seem to be valid."
+ exit 1;
+ fi
+fi
+
+if [ "$(${GIT} rev-parse --is-inside-work-tree 2>/dev/null)" = "true" ]; then
+ IN_GIT_TREE=1
+else
+ IN_GIT_TREE=0
+fi
+
+if [ "${IN_GIT_TREE}" -eq 1 ] && [ -z "${GIT}" ]; then
+ echo "This test needs git to run. Please install it, then run this test again."
+ exit 1
+fi
+
+# Use git ls-files if the code is in a git repo, otherwise use find.
+if [ "${IN_GIT_TREE}" -eq 1 ]; then
+ FIND_FILES="${GIT} ls-files"
+else
+ FIND_FILES="find "
+ FINDOPTS="-type f"
+fi
+
+# Use git grep if the code is in a git repo, otherwise use grep.
+if [ "${IN_GIT_TREE}" -eq 1 ]; then
+ GREP_FILES="${GIT} grep"
+else
+ GREP_FILES="grep -r"
+fi
diff --git a/util/lint/lint-extended-020-signed-off-by b/util/lint/lint-extended-020-signed-off-by
new file mode 100755
index 0000000..ef62a45
--- /dev/null
+++ b/util/lint/lint-extended-020-signed-off-by
@@ -0,0 +1,23 @@
+#!/usr/bin/env sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# DESCR: Check for a signed-off-by line on the latest commit
+
+
+LINTDIR="$(
+ cd -- "$(dirname "$0")" > /dev/null 2>&1 || return
+ pwd -P
+)"
+
+# shellcheck source=helper_functions.sh
+. "${LINTDIR}/helper_functions.sh"
+
+if [ "${IN_GIT_TREE}" -eq 0 ]; then
+ exit 0
+fi
+
+# This test is mainly for the jenkins server
+if ! ${GIT} log -n 1 | grep -q '[[:space:]]\+Signed-off-by: '; then
+ echo "No Signed-off-by line in commit message"
+ exit 1
+fi
--
To view, visit https://review.coreboot.org/c/flashrom/+/70243
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.3.x
Gerrit-Change-Id: I03f1827803f8492d60a0a44174d5822c2265bfcb
Gerrit-Change-Number: 70243
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: newchange
Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70079 )
Change subject: util/lint: Introduce linter for git sign-off-by line
......................................................................
util/lint: Introduce linter for git sign-off-by line
For compliance and to check if the commit message matches the
development guidelines, add a linter checking for the sign-off-by line
in the commit message. Also, hook it up to the test_build.sh script
which is used for CI builds.
These scripts were copied from the coreboot repository and were adjusted
so that they work for flashrom.
Change-Id: I03f1827803f8492d60a0a44174d5822c2265bfcb
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70079
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
---
M test_build.sh
A util/lint/helper_functions.sh
A util/lint/lint-extended-020-signed-off-by
3 files changed, 96 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Edward O'Callaghan: Looks good to me, approved
diff --git a/test_build.sh b/test_build.sh
index 69d3a34..813d0b9 100755
--- a/test_build.sh
+++ b/test_build.sh
@@ -26,6 +26,11 @@
fi
+run_linter() {
+ ./util/lint/lint-extended-020-signed-off-by
+}
+
+
build_make () {
make clean
make -j $(nproc) CONFIG_EVERYTHING=yes
@@ -67,5 +72,7 @@
}
+run_linter
+
build_make
build_meson
diff --git a/util/lint/helper_functions.sh b/util/lint/helper_functions.sh
new file mode 100644
index 0000000..7abdab8
--- /dev/null
+++ b/util/lint/helper_functions.sh
@@ -0,0 +1,45 @@
+#!/usr/bin/env sh
+#
+# SPDX-License-Identifier: GPL-2.0-only
+
+# This file is sourced by the linters so that each one doesn't have to
+# specify these routines individually
+
+LC_ALL=C export LC_ALL
+
+if [ -z "$GIT" ]; then
+ GIT="$(command -v git)"
+else
+ # If git is specified, Do a basic check that it runs and seems like
+ # it's actually git
+ if ! "${GIT}" --version | grep -q git; then
+ echo "Error: ${GIT} does not seem to be valid."
+ exit 1;
+ fi
+fi
+
+if [ "$(${GIT} rev-parse --is-inside-work-tree 2>/dev/null)" = "true" ]; then
+ IN_GIT_TREE=1
+else
+ IN_GIT_TREE=0
+fi
+
+if [ "${IN_GIT_TREE}" -eq 1 ] && [ -z "${GIT}" ]; then
+ echo "This test needs git to run. Please install it, then run this test again."
+ exit 1
+fi
+
+# Use git ls-files if the code is in a git repo, otherwise use find.
+if [ "${IN_GIT_TREE}" -eq 1 ]; then
+ FIND_FILES="${GIT} ls-files"
+else
+ FIND_FILES="find "
+ FINDOPTS="-type f"
+fi
+
+# Use git grep if the code is in a git repo, otherwise use grep.
+if [ "${IN_GIT_TREE}" -eq 1 ]; then
+ GREP_FILES="${GIT} grep"
+else
+ GREP_FILES="grep -r"
+fi
diff --git a/util/lint/lint-extended-020-signed-off-by b/util/lint/lint-extended-020-signed-off-by
new file mode 100755
index 0000000..ef62a45
--- /dev/null
+++ b/util/lint/lint-extended-020-signed-off-by
@@ -0,0 +1,23 @@
+#!/usr/bin/env sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# DESCR: Check for a signed-off-by line on the latest commit
+
+
+LINTDIR="$(
+ cd -- "$(dirname "$0")" > /dev/null 2>&1 || return
+ pwd -P
+)"
+
+# shellcheck source=helper_functions.sh
+. "${LINTDIR}/helper_functions.sh"
+
+if [ "${IN_GIT_TREE}" -eq 0 ]; then
+ exit 0
+fi
+
+# This test is mainly for the jenkins server
+if ! ${GIT} log -n 1 | grep -q '[[:space:]]\+Signed-off-by: '; then
+ echo "No Signed-off-by line in commit message"
+ exit 1
+fi
--
To view, visit https://review.coreboot.org/c/flashrom/+/70079
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I03f1827803f8492d60a0a44174d5822c2265bfcb
Gerrit-Change-Number: 70079
Gerrit-PatchSet: 10
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Felix Singer.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70079 )
Change subject: util/lint: Introduce linter for git sign-off-by line
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS9:
> Unfortunately the +2 got dropped :/
fixed
--
To view, visit https://review.coreboot.org/c/flashrom/+/70079
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I03f1827803f8492d60a0a44174d5822c2265bfcb
Gerrit-Change-Number: 70079
Gerrit-PatchSet: 9
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Comment-Date: Fri, 02 Dec 2022 00:38:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment