From cff3aab4c8162f73854f0b2e2f2a600293eca533 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Sun, 24 Dec 2023 08:27:03 +0000 Subject: [PATCH 1/7] phase2: dlsha2rsyncpl: fix sha2rsync.pl script location MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes following error: File 'sha2rsync.pl' not available at master Fixes: c3ddb0db167d ("phase2: use sha2rsync.pl for 'targetupload'") Signed-off-by: Petr Štetiar --- phase2/master.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phase2/master.cfg b/phase2/master.cfg index da22cce..18236b5 100644 --- a/phase2/master.cfg +++ b/phase2/master.cfg @@ -641,7 +641,7 @@ for arch in arches: factory.addStep(FileDownload( name = "dlsha2rsyncpl", - mastersrc = "sha2rsync.pl", + mastersrc = scripts_dir + "/sha2rsync.pl", workerdest = "../sha2rsync.pl", mode = 0o755, )) From 56f1ba52ca353730c96433d404b781458553c3c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Sat, 23 Dec 2023 07:14:20 +0000 Subject: [PATCH 2/7] phase2: fix stray closing parenthesis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Due to the stray closing parenthesis introduced in commit f0faed2970dd ("phase2: compute checksums") we have currently we've failing checksum step in phase2: argv: b'cd bin/packages/mipsel_24kc_24kf; find . -type f -not -name \'sha256sums\' -printf "%P\n" | sort | xargs -r ../../../staging_dir/host/bin/mkhash -n sha256 | sed -ne \'s!^\\(.*\\) \\(.*\\)$!\x01 *\x02!p\' > sha256sums)' environment: ...snip... using PTY: False /bin/sh: 2: Syntax error: ")" unexpected program finished with exit code 2 Fixes: f0faed2970dd ("phase2: compute checksums") Reported-by: Hannu Nyman Signed-off-by: Petr Štetiar --- phase2/master.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phase2/master.cfg b/phase2/master.cfg index 18236b5..02e7a45 100644 --- a/phase2/master.cfg +++ b/phase2/master.cfg @@ -572,7 +572,7 @@ for arch in arches: description = "Calculating checksums", descriptionDone="Checksums calculated", workdir = "build/sdk", - command = "cd bin/packages/%s; " %(arch[0]) + "find . -type f -not -name 'sha256sums' -printf \"%P\n\" | sort | xargs -r ../../../staging_dir/host/bin/mkhash -n sha256 | sed -ne 's!^\(.*\) \(.*\)$!\1 *\2!p' > sha256sums)", + command = "cd bin/packages/%s; " %(arch[0]) + "find . -type f -not -name 'sha256sums' -printf \"%P\n\" | sort | xargs -r ../../../staging_dir/host/bin/mkhash -n sha256 | sed -ne 's!^\(.*\) \(.*\)$!\1 *\2!p' > sha256sums", haltOnFailure = True )) From e99b029e198cf391ebd26bba4f450331ea491a70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Sat, 23 Dec 2023 06:13:02 +0000 Subject: [PATCH 3/7] Add linting with flake8 tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit flake8 is a Python tool that glues together pycodestyle, pyflakes, mccabe, and third-party plugins to check the style and quality of some Python code. Currently we've issue in phase2 in checksum step: /bin/sh: 2: Syntax error: ")" unexpected And it seems, that flake8 is able to spot places which might lead to such issues during runtime: phase2/master.cfg:733:151: W605 invalid escape sequence '\(' phase2/master.cfg:733:155: W605 invalid escape sequence '\)' phase2/master.cfg:733:158: W605 invalid escape sequence '\(' phase2/master.cfg:733:162: W605 invalid escape sequence '\)' So lets enable flake8 checking on the CI so we can spot similar places in the future and address them before deployment. We dont want to make current ongoing work on phase2 code harder, thus we don't touch that part yet, so we whitelist most of the checks for now. References: f0faed2970dd ("phase2: compute checksums") Signed-off-by: Petr Štetiar --- .flake8 | 4 ++++ .github/workflows/build-push.yml | 3 +++ .gitignore | 1 + phase1/master.cfg | 41 ++++++++++++++++++++++---------- requirements-dev.txt | 1 + 5 files changed, 38 insertions(+), 12 deletions(-) create mode 100644 .flake8 diff --git a/.flake8 b/.flake8 new file mode 100644 index 0000000..66380a1 --- /dev/null +++ b/.flake8 @@ -0,0 +1,4 @@ +[flake8] +max-line-length = 140 +per-file-ignores = + phase2/master.cfg: E101,E117,E128,E201,E202,E203,E221,E225,E251,E266,E302,E305,E501,W191 diff --git a/.github/workflows/build-push.yml b/.github/workflows/build-push.yml index 4f5b3f4..1ae0f27 100644 --- a/.github/workflows/build-push.yml +++ b/.github/workflows/build-push.yml @@ -40,6 +40,9 @@ jobs: - name: Lint with ruff run: ruff phase*/master.cfg + - name: Lint with flake8 + run: flake8 phase*/master.cfg + - name: Stylecheck with black run: black phase1/master.cfg diff --git a/.gitignore b/.gitignore index 0d4e15f..5c68e09 100644 --- a/.gitignore +++ b/.gitignore @@ -27,3 +27,4 @@ phase[12]/twistd.* !.ruff.toml !tests !tests/**/* +!.flake8 diff --git a/phase1/master.cfg b/phase1/master.cfg index 2d6f304..e9c6b71 100644 --- a/phase1/master.cfg +++ b/phase1/master.cfg @@ -107,7 +107,7 @@ pb_port = inip1.get("port") or 9989 # a shorter alias to save typing. c = BuildmasterConfig = {} -####### PROJECT IDENTITY +# PROJECT IDENTITY # the 'title' string will appear at the top of this buildbot # installation's html.WebStatus home page (linked to the @@ -124,7 +124,7 @@ c["titleURL"] = ini["general"].get("title_url") c["buildbotURL"] = inip1.get("buildbot_url") -####### BUILDWORKERS +# BUILDWORKERS # The 'workers' list defines the set of recognized buildworkers. Each element is # a Worker object, specifying a unique worker name and password. The same @@ -292,7 +292,7 @@ def prioritizeBuilders(master, builders): c["prioritizeBuilders"] = prioritizeBuilders -####### CHANGESOURCES +# CHANGESOURCES # find targets targets = dict() @@ -358,7 +358,7 @@ c["change_source"].append( ) ) -####### SCHEDULERS +# SCHEDULERS # Configure the Schedulers, which decide how to react to incoming changes. @@ -541,7 +541,7 @@ c["schedulers"].append( schedulers.Triggerable(name="trigger", builderNames=builderNames, priority=20) ) -####### BUILDERS +# BUILDERS # The 'builders' list defines the Builders, which tell Buildbot how to perform a build: # what steps, and which workers can execute them. Note that any particular build will @@ -882,7 +882,11 @@ def prepareFactory(target): name="gitverify", description="Ensuring that Git HEAD is pointing to a branch or tag", descriptionDone="Git HEAD is sane", - command='git rev-parse --abbrev-ref HEAD | grep -vxqF HEAD || git show-ref --tags --dereference 2>/dev/null | sed -ne "/^$(git rev-parse HEAD) / { s|^.*/||; s|\\^.*||; p }" | grep -qE "^v[0-9][0-9]\\."', + command=( + "git rev-parse --abbrev-ref HEAD | grep -vxqF HEAD || " + "git show-ref --tags --dereference 2>/dev/null | sed -ne " + '"/^$(git rev-parse HEAD) / { s|^.*/||; s|\\^.*||; p }" | grep -qE "^v[0-9][0-9]\\."' + ), haltOnFailure=True, ) ) @@ -944,7 +948,9 @@ def prepareFactory(target): name="newconfig", descriptionDone=".config seeded", command=Interpolate( - "printf 'CONFIG_TARGET_%(kw:target)s=y\\nCONFIG_TARGET_%(kw:target)s_%(kw:subtarget)s=y\\nCONFIG_SIGNED_PACKAGES=%(kw:usign:#?|y|n)s\\n' >> .config", + "printf 'CONFIG_TARGET_%(kw:target)s=y\\n" + "CONFIG_TARGET_%(kw:target)s_%(kw:subtarget)s=y\\n" + "CONFIG_SIGNED_PACKAGES=%(kw:usign:#?|y|n)s\\n' >> .config", target=target, subtarget=subtarget, usign=GetUsignKey, @@ -1145,7 +1151,11 @@ def prepareFactory(target): name="kernelversion", property="kernelversion", description="Finding the effective Kernel version", - command="make --no-print-directory -C target/linux/ val.LINUX_VERSION val.LINUX_RELEASE val.LINUX_VERMAGIC | xargs printf '%s-%s-%s\\n'", + command=( + "make --no-print-directory -C target/linux/ " + "val.LINUX_VERSION val.LINUX_RELEASE val.LINUX_VERMAGIC | " + "xargs printf '%s-%s-%s\\n'" + ), env={"TOPDIR": Interpolate("%(prop:builddir)s/build")}, ) ) @@ -1341,7 +1351,10 @@ def prepareFactory(target): description="Packing files to sign", descriptionDone="Files to sign packed", command=Interpolate( - "find bin/targets/%(kw:target)s/%(kw:subtarget)s%(prop:libc)s/ bin/targets/%(kw:target)s/%(kw:subtarget)s%(prop:libc)s/kmods/ -mindepth 1 -maxdepth 2 -type f -name sha256sums -print0 -or -name Packages -print0 | xargs -0 tar -czf sign.tar.gz", + "find bin/targets/%(kw:target)s/%(kw:subtarget)s%(prop:libc)s/ " + "bin/targets/%(kw:target)s/%(kw:subtarget)s%(prop:libc)s/kmods/ " + "-mindepth 1 -maxdepth 2 -type f -name sha256sums -print0 -or " + "-name Packages -print0 | xargs -0 tar -czf sign.tar.gz", target=target, subtarget=subtarget, ), @@ -1674,7 +1687,11 @@ def prepareFactory(target): name="sourcelist", description="Finding source archives to upload", descriptionDone="Source archives to upload found", - command="find dl/ -maxdepth 1 -type f -not -size 0 -not -name '.*' -not -name '*.hash' -not -name '*.dl' -newer .config -printf '%f\\n' > sourcelist", + command=( + "find dl/ -maxdepth 1 -type f -not -size 0 " + "-not -name '.*' -not -name '*.hash' -not -name " + "'*.dl' -newer .config -printf '%f\\n' > sourcelist" + ), haltOnFailure=True, ) ) @@ -1775,7 +1792,7 @@ for brname in branchNames: ) -####### STATUS TARGETS +# STATUS TARGETS # 'status' is a list of Status Targets. The results of each build will be # pushed to these targets. buildbot/status/*.py has a variety to choose from, @@ -1826,7 +1843,7 @@ c["revlink"] = util.RevlinkMatch( r"https://git.openwrt.org/?p=openwrt/\1.git;a=commit;h=%s", ) -####### DB URL +# DB URL c["db"] = { # This specifies what database buildbot uses to store its state. You can leave diff --git a/requirements-dev.txt b/requirements-dev.txt index e1cb65f..033af6b 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,3 +1,4 @@ cram==0.7 black==23.3.0 ruff==0.0.267 +flake8==6.1.0 From 0b70ae69c4ef440de87863db5c4502367638c6ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Sun, 24 Dec 2023 09:30:58 +0000 Subject: [PATCH 4/7] phase2: checksums: split long command line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To make it more readable, diffs smaller, easier to review etc. Signed-off-by: Petr Štetiar --- phase2/master.cfg | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/phase2/master.cfg b/phase2/master.cfg index 02e7a45..fa585d7 100644 --- a/phase2/master.cfg +++ b/phase2/master.cfg @@ -572,7 +572,10 @@ for arch in arches: description = "Calculating checksums", descriptionDone="Checksums calculated", workdir = "build/sdk", - command = "cd bin/packages/%s; " %(arch[0]) + "find . -type f -not -name 'sha256sums' -printf \"%P\n\" | sort | xargs -r ../../../staging_dir/host/bin/mkhash -n sha256 | sed -ne 's!^\(.*\) \(.*\)$!\1 *\2!p' > sha256sums", + command = "cd bin/packages/%s; " %(arch[0]) + + "find . -type f -not -name 'sha256sums' -printf \"%P\n\" | " + + "sort | xargs -r ../../../staging_dir/host/bin/mkhash -n sha256 | " + + "sed -ne 's!^\(.*\) \(.*\)$!\1 *\2!p' > sha256sums", haltOnFailure = True )) From 0f42b182f8e4f7af22121ed6d206cd4d872af7bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Sat, 23 Dec 2023 07:33:21 +0000 Subject: [PATCH 5/7] phase2: fix invalid escape sequence flake8 warning in checksums step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolved a flake8 linting warning related to an invalid escape sequence in the ShellCommand for calculating checksums: phase2/master.cfg:739:28: W605 invalid escape sequence '\(' phase2/master.cfg:739:32: W605 invalid escape sequence '\)' phase2/master.cfg:739:35: W605 invalid escape sequence '\(' phase2/master.cfg:739:39: W605 invalid escape sequence '\)' The warning was caused by the use of unescaped parentheses in a regular expression within a sed command. Use a raw string (with an 'r' prefix) to treat backslashes as literal characters, ensuring that the regular expression is correctly interpreted and flake8 does not raise a warning. This fix ensures that the code adheres to Python's string handling best practices and maintains the integrity of the regular expression functionality. Fixes: f0faed2970dd ("phase2: compute checksums") Signed-off-by: Petr Štetiar --- phase2/master.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phase2/master.cfg b/phase2/master.cfg index fa585d7..deebc28 100644 --- a/phase2/master.cfg +++ b/phase2/master.cfg @@ -575,7 +575,7 @@ for arch in arches: command = "cd bin/packages/%s; " %(arch[0]) + "find . -type f -not -name 'sha256sums' -printf \"%P\n\" | " + "sort | xargs -r ../../../staging_dir/host/bin/mkhash -n sha256 | " - + "sed -ne 's!^\(.*\) \(.*\)$!\1 *\2!p' > sha256sums", + + r"sed -ne 's!^\(.*\) \(.*\)$!\1 *\2!p' > sha256sums", haltOnFailure = True )) From 86864480eb7a25650c136fc5f97ae8ac7c52255b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Sat, 23 Dec 2023 06:43:02 +0000 Subject: [PATCH 6/7] requirements-dev: use black 23.12.1 and ruff 0.1.9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix ruff's recommendation: phase2/master.cfg:328:16: E721 Do not compare types, use `isinstance()` "Unlike a direct type comparison, isinstance will also check if an object is an instance of a class or a subclass thereof." Signed-off-by: Petr Štetiar --- phase1/master.cfg | 3 ++- phase2/master.cfg | 2 +- requirements-dev.txt | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/phase1/master.cfg b/phase1/master.cfg index e9c6b71..e29bb9d 100644 --- a/phase1/master.cfg +++ b/phase1/master.cfg @@ -266,7 +266,8 @@ def prioritizeBuilders(master, builders): pos = 99 for name, prio in bldrNamePrio.items(): if bldr.name.startswith(name): - pos = prio + 50 - min(hiprio, 50) # higher priority (larger positive number) raises position + # higher priority (larger positive number) raises position + pos = prio + 50 - min(hiprio, 50) break # pos order: janitor/local (0), tag builds if any [1..50], !tag builds [51...] diff --git a/phase2/master.cfg b/phase2/master.cfg index deebc28..007bc11 100644 --- a/phase2/master.cfg +++ b/phase2/master.cfg @@ -280,7 +280,7 @@ def IsArchitectureSelected(target): def CheckArchitectureProperty(step): try: options = step.getProperty("options") - if type(options) is dict: + if isinstance(options, dict): selected_arch = options.get("architecture", "all") if selected_arch != "all" and selected_arch != target: return False diff --git a/requirements-dev.txt b/requirements-dev.txt index 033af6b..e373155 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,4 +1,4 @@ cram==0.7 -black==23.3.0 -ruff==0.0.267 +black==23.12.1 +ruff==0.1.9 flake8==6.1.0 From fe54cb8d30e5e7e1509c185449885f96d934ea30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Sun, 24 Dec 2023 19:14:14 +0000 Subject: [PATCH 7/7] phase2: fix relative paths for scripts and files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently `buildlist` step fails with following: ../../../sha2rsync.pl ../../arch-sha256sums bin/packages/aarch64_generic/sha256sums rsynclist in dir /builder/aarch64_generic/build/sdk (timeout 1200 secs) watching logfiles {} argv: [b'../../../sha2rsync.pl', b'../../arch-sha256sums', b'bin/packages/aarch64_generic/sha256sums', b'rsynclist'] environment: ... PWD=/builder/aarch64_generic/build/sdk ... Upon execvpe b'../../../sha2rsync.pl' [b'../../../sha2rsync.pl', b'../../arch-sha256sums', b'bin/packages/aarch64_generic/sha256sums', b'rsynclist'] in environment id 139847367136832 :Traceback (most recent call last): ... FileNotFoundError: [Errno 2] No such file or directory: b'../../../sha2rsync.pl' due to relative paths being off by one: worker work dir = /builder/aarch64_cortex-a72/build workerdest = "../sha2rsync.pl" is absolute path /builder/aarch64_cortex-a72/sha2rsync.pl thus relative path from: FileNotFoundError: [Errno 2] No such file or directory: b'../../../sha2rsync.pl' in following context: PWD=/builder/aarch64_generic/build/sdk b'../../../sha2rsync.pl' is wrong absolute path `/builder/sha2rsync.pl` by one directory level, thus adjust all those paths accordingly. Fixes: c3ddb0db167d ("phase2: use sha2rsync.pl for 'targetupload'") Signed-off-by: Petr Štetiar --- phase2/master.cfg | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/phase2/master.cfg b/phase2/master.cfg index 007bc11..23a67f7 100644 --- a/phase2/master.cfg +++ b/phase2/master.cfg @@ -653,7 +653,7 @@ for arch in arches: name = "buildlist", description = "Building list of files to upload", workdir = "build/sdk", - command = ["../../../sha2rsync.pl", "../../arch-sha256sums", "bin/packages/%s/sha256sums" %(arch[0]), "rsynclist"], + command = ["../../sha2rsync.pl", "../arch-sha256sums", "bin/packages/%s/sha256sums" %(arch[0]), "rsynclist"], haltOnFailure = True, )) @@ -671,7 +671,7 @@ for arch in arches: name = "packageupload", description = "Uploading package files", workdir = "build/sdk", - command = ["../../../rsync.sh"] + rsync_defopts + ["--files-from=rsynclist", "--delay-updates", "--partial-dir=.~tmp~%s" %(arch[0]), "-a", "bin/packages/%s/" %(arch[0]), Interpolate("%(kw:rsyncbinurl)s/packages%(kw:suffix)s/%(kw:archname)s/", rsyncbinurl=rsync_bin_url, suffix=GetDirectorySuffix, archname=arch[0])], + command = ["../../rsync.sh"] + rsync_defopts + ["--files-from=rsynclist", "--delay-updates", "--partial-dir=.~tmp~%s" %(arch[0]), "-a", "bin/packages/%s/" %(arch[0]), Interpolate("%(kw:rsyncbinurl)s/packages%(kw:suffix)s/%(kw:archname)s/", rsyncbinurl=rsync_bin_url, suffix=GetDirectorySuffix, archname=arch[0])], env={'RSYNC_PASSWORD': rsync_bin_key}, haltOnFailure = True, logEnviron = False @@ -681,7 +681,7 @@ for arch in arches: name = "packageprune", description = "Pruning package files", workdir = "build/sdk", - command = ["../../../rsync.sh"] + rsync_defopts + ["--delete", "--existing", "--ignore-existing", "--delay-updates", "--partial-dir=.~tmp~%s" %(arch[0]), "-a", "bin/packages/%s/" %(arch[0]), Interpolate("%(kw:rsyncbinurl)s/packages%(kw:suffix)s/%(kw:archname)s/", rsyncbinurl=rsync_bin_url, suffix=GetDirectorySuffix, archname=arch[0])], + command = ["../../rsync.sh"] + rsync_defopts + ["--delete", "--existing", "--ignore-existing", "--delay-updates", "--partial-dir=.~tmp~%s" %(arch[0]), "-a", "bin/packages/%s/" %(arch[0]), Interpolate("%(kw:rsyncbinurl)s/packages%(kw:suffix)s/%(kw:archname)s/", rsyncbinurl=rsync_bin_url, suffix=GetDirectorySuffix, archname=arch[0])], env={'RSYNC_PASSWORD': rsync_bin_key}, haltOnFailure = True, logEnviron = False @@ -721,7 +721,7 @@ for arch in arches: name = "logupload", description = "Uploading failure logs", workdir = "build/sdk", - command = ["../../../rsync.sh"] + rsync_defopts + ["--delete", "--delay-updates", "--partial-dir=.~tmp~%s" %(arch[0]), "-az", "faillogs/", Interpolate("%(kw:rsyncbinurl)s/faillogs%(kw:suffix)s/%(kw:archname)s/", rsyncbinurl=rsync_bin_url, suffix=GetDirectorySuffix, archname=arch[0])], + command = ["../../rsync.sh"] + rsync_defopts + ["--delete", "--delay-updates", "--partial-dir=.~tmp~%s" %(arch[0]), "-az", "faillogs/", Interpolate("%(kw:rsyncbinurl)s/faillogs%(kw:suffix)s/%(kw:archname)s/", rsyncbinurl=rsync_bin_url, suffix=GetDirectorySuffix, archname=arch[0])], env={'RSYNC_PASSWORD': rsync_bin_key}, haltOnFailure = False, flunkOnFailure = False, @@ -742,7 +742,7 @@ for arch in arches: name = "sourceupload", description = "Uploading source archives", workdir = "build/sdk", - command = ["../../../rsync.sh"] + rsync_defopts + ["--files-from=sourcelist", "--size-only", "--delay-updates", + command = ["../../rsync.sh"] + rsync_defopts + ["--files-from=sourcelist", "--size-only", "--delay-updates", Interpolate("--partial-dir=.~tmp~%(kw:archname)s~%(prop:workername)s", archname=arch[0]), "-a", "dl/", "%s/" %(rsync_src_url)], env={'RSYNC_PASSWORD': rsync_src_key}, haltOnFailure = False,