From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id sTWpEbrMOGp8/hIAWB0awg (envelope-from ) for ; Mon, 22 Jun 2026 01:48:42 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=OjZgpqCh; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 429BE1E098; Mon, 22 Jun 2026 01:48:42 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 4F6121E070 for ; Mon, 22 Jun 2026 01:48:41 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D8DC34BA2E36 for ; Mon, 22 Jun 2026 05:48:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D8DC34BA2E36 Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=OjZgpqCh Received: from mail-qk1-x72f.google.com (mail-qk1-x72f.google.com [IPv6:2607:f8b0:4864:20::72f]) by sourceware.org (Postfix) with ESMTPS id E94CB4BA2E09 for ; Mon, 22 Jun 2026 05:48:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E94CB4BA2E09 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E94CB4BA2E09 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::72f ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1782107294; cv=none; b=BSVT7EDjBzYL/oWF85OLZHfQmkk/PXecTfC0RNlfNi4sdt7tcWrulshsjkIG00CrZad6mQvPX9oD92E35iJA11/QXo5peMZYFqr6eQcVeyYVAEUOvZk/v4ZCetXL9kyjnXECa6BwvHfpG3zJfWsLhZamoSC0d1wylOj7B3ZL4g8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1782107294; c=relaxed/simple; bh=rHMn7SYO5aC3jvfYKxhKr7fIKhj+tkDNfPtzrX8xZt0=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Vo7sPP7PucdGahfD2DFvOxi24ReCO9iRPyhylqDoirLZD66l7lp6uNcX+jOMjgP082gFqkRThoUruyWVlvNzFlmGT4uwLSlhUjIq2KlqXqBbQNGf4zq/WcSXXS8EdzBfV+v79DgnkgV0FbHOLV2zqyY9iwa1UqMJBRi3uqKYcRI= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=OjZgpqCh DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E94CB4BA2E09 Received: by mail-qk1-x72f.google.com with SMTP id af79cd13be357-922951391d6so123305685a.0 for ; Sun, 21 Jun 2026 22:48:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1782107293; x=1782712093; darn=sourceware.org; h=mime-version:message-id:date:user-agent:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=9shQZbJxfwMCkFmK7YOp9eEx8vTnAFGV9iux5owc/qE=; b=OjZgpqCh7uUx29TcSzYghJwHK5kVdcJU4PB0/wSesmHRxEOgc3UIHACZsEWGzVAJuH ZArK+oQU/jOO5dCdgA4pOiIpdl69BiKIruB1NT048j8gCaL1FwKMNnHew/7twwbXm5wF gznInCCU1YTaTp8H84AFJhNOr9INLLQSKPMPGmDPQTfS4AT07PKRP7d4wOgAgXu1qwss YSKwUENh5aPHp+OFwOQe2fM7m/YFOglTYoYPDWzFUxs5ZS8IYL/hAbs/YWq4yElau3ol bgr08JTLz4eVnt4DIVyrxtcTXjUPQfuezfYz5d8SRWPE15b7iu1BUy4ykNlOHtCADAvH zMwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782107293; x=1782712093; h=mime-version:message-id:date:user-agent:references:in-reply-to :subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=9shQZbJxfwMCkFmK7YOp9eEx8vTnAFGV9iux5owc/qE=; b=Vb9CR1v+qCIn3baRioDva8xfkg+J7sLO9oH4ZkgGZXYBh4TFIhNeT0NMlhqhmFBEAV ei+X5Sz6CIeZK3EZk561299fbRFi6/7btu9UMtwq+vxowRHQ7kkouNpViwl9I1ACyrAp oLYCrpc46szPzSUAo+37HI22iOlUJg0tKeT8z0oBa6n8SajT79T4aFY3Cevn77b3XPn+ G6xF3rBpeN0/oS8uR3908aBkU1oIifjtAWsKiQyTo51jt6MbqF8sLOOUE2dq7DDY7lAQ jlLeNKyseMj11/f+tUNOMOihVoOT5fo9fwnznek3fIWgChX/CQkt2cduiUKATns1Cku/ VVEw== X-Forwarded-Encrypted: i=1; AFNElJ/0TBjqTZKAtYCTObz4xddQ1S8qdvZQapmc79EvNknk0D+uppn+gUTx8NdE8LIC3Kxpbkj9egal53ipDQ==@sourceware.org X-Gm-Message-State: AOJu0YzVdjrsYnd8M8/AxRuzF/URPxRVSNJErQEpSh6s6LmNhRAT0mRo m3VGymsQ5ESOJuohxJIKcz85SdXkGSWSpEkJ27IAWlm2JZPWhmb0RdIYP+M6s4zvtpAz9/HS0UN llwVZcfg= X-Gm-Gg: AfdE7cmAw93jgPtu4kzDvHXcpu7Ne4Off0MDDtwMGg/VOPtpuNHLSU5kPoGpibKTvsu OZJulD7NCUhcxgBJuiJW77qfF/HEC1odwsSi16eUEvu6k9newykViGrm0TvAuq4ZUpezptK9Gh1 3KyDfnNyAo56exON3GMxPfQ1uIoVgjgbtolwhv+9FE48x989st/grhqTv0bnUv7ai8dIlvUriBX DkeqJJwMhIJEKF33oNRedknd065P58my/irCpqTpkMFmV+yyoLbniET88wxVk99uyIJM8FWRSDr Gm5eGiskrrwKHu6T46avdXvVSF8yyc9dZ/W5J2fvwPAFj7nqRpnU8ZUvYZWb3Zy5PPLmekJXfda NC20PefHD7x2e7krjpiGPW33hRuwAVnnXQrttJeATiND/Pu5Z1+LUXz2zzx84SdKSIQZ+eeDNkX Q8tCvWifOJVEZE28klfjqlCPM= X-Received: by 2002:a05:620a:370d:b0:915:bf79:3e09 with SMTP id af79cd13be357-920910a6012mr2328163185a.44.1782107293016; Sun, 21 Jun 2026 22:48:13 -0700 (PDT) Received: from localhost ([2804:14d:7e39:8083:33bc:f32e:9aa5:b915]) by smtp.gmail.com with ESMTPSA id af79cd13be357-921daa8074csm800905585a.25.2026.06.21.22.48.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 21 Jun 2026 22:48:12 -0700 (PDT) From: Thiago Jung Bauermann To: Simon Marchi Cc: Tom de Vries , gdb-patches@sourceware.org Subject: Re: [PATCH] [pre-commit] Add shellcheck In-Reply-To: (Simon Marchi's message of "Thu, 18 Jun 2026 13:05:03 -0400") References: <20260618151957.76500-1-tdevries@suse.de> User-Agent: mu4e 1.14.2; emacs 30.2 Date: Mon, 22 Jun 2026 05:48:09 +0000 Message-ID: <874iivunvq.fsf@linaro.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org --=-=-= Content-Type: text/plain Simon Marchi writes: > On 2026-06-18 11:19, Tom de Vries wrote: >> I found a pure python implementation of shellcheck [1]. >> >> Use it to run shellcheck on scripts in the repo. >> >> Exclude any scripts that are not currently clean. >> >> Running it seems reasonably fast: >> ... >> $ pre-commit run shellcheck --all-files -v >> shellcheck...............................................................Passed >> - hook id: shellcheck >> - duration: 0.06s >> ... >> >> For information on other solutions, see this RFC [2]. >> >> [1] https://pypi.org/project/pureshellcheck/0.2.2/ >> [2] https://sourceware.org/pipermail/gdb-patches/2024-November/213400.html > > While I'm sympathetic to the use of pre-commit (I added the first > hooks), I'm starting to get a bit worried that we kind of blindly pull > hooks from random places without paying much attention. This is going > to get executed on the machines of many GDB devs, and probably some CI > too. > > I had this thought because this one has the "random project on github > vibes" (it appears to be a vibe coded project started a week ago). More > established projects (like black) are not immune to being compromised, > but they are easier to trust I guess. I have the same kind of concern, but I'm more paranoid. As a general principle I prefer to install distro packages rather than language-specific packages (such as the ones from PyPI), on the assumption that distros tend to be more careful about incorporating packages and updating them (it's a weak assumption though). Because of that, in order to run the pre-commit checks I created a script (attached) that parses .pre-commit-config.yaml and runs the hooks with the tools that are already installed on the system. It doesn't try to download anything. Today I added support for running each hook in an isolated container with access only to the binutils-gdb repo, and in read-only mode at that. It uses Guix to set up the containers so when Guix isn't installed you need to pass the --no-container option. Also when using containers the script can only use the tools packaged in Guix, which currently doesn't have some of the tools referenced by our .pre-commit-config.yaml, or has older versions of them. Another bad news is that the script is in Scheme. :) But you get the idea. :) -- Thiago (he/him) --=-=-= Content-Type: text/scheme; charset=utf-8 Content-Disposition: inline; filename=run-gdb-precommit-hooks Content-Transfer-Encoding: quoted-printable #!/usr/bin/env -S guile -e main -s !# ;;; coding: utf-8 (use-modules (ice-9 control) (ice-9 ftw) (ice-9 getopt-long) (ice-9 popen) (ice-9 rdelim) (ice-9 regex) (srfi srfi-1) ; List libray (srfi srfi-43) ; Vector library (yaml)) (define %prog-name "run-gdb-precommit-hooks") ;; All my GDB working trees have a path starting with this string. (define %gdb-repo-prefix (string-append (getenv "HOME") "/src/binutils-gdb"= )) (define *ignore-tool-checks?* #f) (define *use-container?* #f) (define (message message . arguments) (apply format (current-output-port) (string-append %prog-name ": " messag= e "\n") arguments) #t) (define (error-message message . arguments) (apply format (current-error-port) (string-append %prog-name ": Error: " = message "\n") arguments) #f) (define (warning-message message . arguments) (apply format (current-error-port) (string-append %prog-name ": Warning: = " message "\n") arguments) #t) (define (error-or-warning message . arguments) (apply (if *ignore-tool-checks?* warning-message error-message) message a= rguments)) (define (get-gdb-repo-path) "Assuming CWD is in a GDB repo, return its root." (let* ((cwd (getcwd)) (first-/ (string-index cwd #\/ (string-length %gdb-repo-prefix)))) (substring cwd 0 (or first-/ (string-length cwd))))) (define* (run-cmd args #:key (output #f) (guix-packages #f) (container? #t)) "Run given command =E2=80=94 inside a container if CONTAINER? is #t. If OUTPUT is a string, it's the path of a file to which output will be r= edirected. The return value depends on the OUTPUT parameter: - if it's #t, return command output as a string. - Otherwise, return boolean indicating whether the command succeeded or = failed." (when (and *use-container?* container?) (set! args (append (list "guix" "shell" "--container" "--emulate-fhs" (string-append "--expose=3D" (get-gdb-repo-pat= h)) ;; Many local hooks use the folowing packages,= so just make ;; them always available in the container. "bash" "coreutils" "git" "python") guix-packages (list "--") args))) (false-if-exception (if (boolean? output) (if output ;; Return command output as a string. (call-with-port (apply open-pipe* OPEN_READ args) read-line) ;; Return boolean indicating success or failure. (eq? (status:exit-val (cdr (waitpid (spawn (list-ref args 0) args)))) 0)) (if (string? output) ;; Save command output to file. (eq? (status:exit-val (call-with-output-file output (lambda (output-port) (cdr (waitpid (spawn (list-ref args 0) args #:output o= utput-port)))))) 0) ;; OUTPUT is an invalid value. #f)))) (define (read-pre-commit-config file) "Read .pre-commit-config.yaml. Returns list of association lists describing each hook, or #f if an unrecognized hook is found and *ignore-tool-checks?* is false." (define (get-hook-info id) "Obtain information about hook ID. The pre-commit tools gets most of this information from each hook's .pre-commit-hooks.yaml. We simply hard-code it here." (define hook-infos (list `((id . "black") (get-version . ,(=CE=BB () ;; Example output: ;; .black-real, 25.1.0 (compiled: no) (list-ref (string-tokenize (run-cmd '("black" "-= -version") #:guix-packa= ges '("python-black") #:output #t)) 1))) (entry . "black") (guix-packages . ("python-black"))) `((id . "flake8") (get-version . ,(=CE=BB () ;; Example output: ;; 7.1.1 (mccabe: 0.7.0, pycodestyle: 2.12.1, py= flakes: 3.2.0) CPython 3.11.14 on Linux (list-ref (string-tokenize (run-cmd '("flake8" "= --version") #:guix-packa= ges '("python-flake8") #:output #t)) 0))) (entry . "flake8") (guix-packages . ("python-flake8"))) `((id . "isort") (get-version . ,(=CE=BB () ;; Example output: ;; 6.0.1 (run-cmd '("isort" "--version-number") #:guix-packages '("python-isort") #:output #t))) (entry . "isort") (args . ,(if *use-container?* ;; Use --check-only because the container can't write= to the repo. '("--check-only" "--filter-files") '("--filter-files"))) (guix-packages . ("python-isort"))) `((id . "codespell") (get-version . ,(=CE=BB () ;; Example output: ;; 2.3.0 (string-append "v" (run-cmd '("codespell" "--ver= sion") #:guix-packages '("p= ython-codespell") #:output #t)))) (entry . "codespell") (guix-packages . ("python-codespell"))) `((id . "tclint") (get-version . ,(=CE=BB () ;; FIXME: For some reason, my tclint Guix packag= e has this output for ;; tclint --version: "tclint (unknown version)",= so for now get the ;; version number from Guix. ;; The yaml's rev field has a v in the beginning. ;; Example output: ;; tclint 0.8.0 out /gnu/store/=E2=80=A6-= tclint-0.8.0 (string-append "v" (list-ref (string-tokenize (run-cmd '("guix" "pac= kage" "-I" "tclint") #:guix-packag= es '("tclint") ;; There's no= Guix inside the ;; container. #:container? = #f #:output #t)) 1)))) (entry . "tclint") (guix-packages . ("tclint"))) `((id . "check-file-mode") (guix-packages . ("grep"))) `((id . "check-gnu-style") (guix-packages . ("python-termcolor" "python-unidiff"))))) (fold (=CE=BB (hook-info result) (or result (if (equal? (assoc-ref hook-info 'id) id) hook-info #f))) #f hook-infos)) (define (yaml-vector->symbols-list types-from-yaml) "Convert vector of strings to list of symbols." (if types-from-yaml (vector-fold (=CE=BB (_ result string) (cons (string->symbol string) result)) '() types-from-yaml) '())) (let* ((yaml-file (read-yaml-file file)) (default-stages (yaml-vector->symbols-list (assoc-ref yaml-file "d= efault_stages")))) ;; Loop through the vector of repos, building the list of association l= ists on the way. (vector-fold (=CE=BB (_ result repo) ;; Loop through the repo's vector of hooks, building the= list of ;; association lists on the way. (vector-fold (=CE=BB (_ hooks-result hook) (let* ((id (assoc-ref hook "id")) (hook-info (get-hook-info id)) (local? (string=3D? (assoc-ref rep= o "repo") "local")) (rev (assoc-ref repo "rev"))) (if (and hooks-result (or hook-info loc= al?)) (let* ((get-version (assoc-ref hook= -info 'get-version)) (entry (or (assoc-ref hook "= entry") (assoc-ref hook-i= nfo 'entry))) (types-or (yaml-vector->symb= ols-list (assoc-ref hook "= types_or"))) (files (assoc-ref hook "file= s")) (exclude (assoc-ref hook "ex= clude")) (info-args (or (assoc-ref ho= ok-info 'args) '())) (hook-args (vector->list (or= (assoc-ref hook "args") = (make-vector 0)))) (guix-packages (or (assoc-re= f hook-info 'guix-packages) '())) (hook-stages (assoc-ref hook= "stages")) (stages (or (and hook-stages (yaml-vector->= symbols-list hook-stages)) default-stages))) ;; Append this hook's association= list to the result list. (append hooks-result (list (list (cons 'id id) (cons 'version-req= uired rev) (cons 'get-version= get-version) (cons 'entry entry) (cons 'types-or ty= pes-or) (cons 'files files) (cons 'args (appen= d info-args hook-args)) (cons 'guix-packag= es guix-packages) (cons 'stages stag= es) (cons 'exclude exc= lude))))) (begin (unless hook-info (error-or-warning "Unknown hook= ~a." id)) (if *ignore-tool-checks?* hooks-r= esult #f))))) result (assoc-ref repo "hooks"))) '() (assoc-ref yaml-file "repos")))) (define* (filter-repo-files #:key (files #f) (exclude #f) (types-or '())) "Arguments: - If provided, FILES and EXCLUDE should be compiled regexp structures. Returns list of files matching constraints." ;; Always enter subdirectories. (define (enter-dir? path stat result) result) (define (leaf path stat result) "Call FUNCTION on PATH if the filters allow it." (let ((adjusted-path (if (string-prefix? "./" path) ;; Remove "./" from beginning of path. (substring path 2) path))) (if (and (or (not (member 'file types-or)) (eq? (stat:type stat) 'regular= )) (or (not files) (not (null? (list-matches files adjusted-path)))) (or (not exclude) (null? (list-matches exclude adjusted-path)))) (cons adjusted-path result) result))) ;; Don't do anything with directories, skipped or otherwise. (define (down path stat result) result) (define (up path stat result) result) (define (skip path stat result) result) ;; Report unreadable files/directories, but keep going. (define (error path stat errno result) (warning-message "~a: ~a" path (strerror errno)) result) (file-system-fold enter-dir? leaf down up skip error '() ".")) (define (print-help) (display "\ Run the GDB pre-commit hooks without using the pre-commit tool. Options: -h, --help Show this help message. -i, --ignore-tool-checks Ignore required tool versions or unknown tools (still prints a warning). -n, --no-container Don't run hooks in isolated containers.\n")) (define (main args) (exit (let/ec return (let* ((option-spec '((help (single-char #\h)) (ignore-tool-checks (single-char #\i)) (no-container (single-char #\n)))) (options (getopt-long args option-spec)) (help? (option-ref options 'help #f)) (ignore-tool-checks? (option-ref options 'ignore-tool-checks #f= )) (container? (not (option-ref options 'no-container #f))) (non-options (option-ref options '() '()))) (when (or help? (> (length non-options) 0)) (print-help) (return (if help? 0 3))) (unless (string-prefix? %gdb-repo-prefix (getcwd)) (error-message "Needs to be run inside a GDB repository.") (return 3)) (set! *ignore-tool-checks?* ignore-tool-checks?) (set! *use-container?* (and container? ;; We can only use containers if the gui= x command is ;; available. (run-cmd '("guix" "--version") #:output "/dev/null" #:container? #f))) ;; If Guix isn't available, fail unless "--no-container" has been gi= ven. (when (and container? (not *use-container?*)) (error-message "Use of containers requested but Guix isn't availab= le.") (return 3)) ;; Run hooks from the root of the GDB repository. (chdir (get-gdb-repo-path)) (let ((hooks (read-pre-commit-config ".pre-commit-config.yaml"))) (unless hooks (return 3)) (fold (=CE=BB (hook previous) (let/ec return-hook ;; Skip hook if it should only run on commit-msg stage. ;; This is for one of the codespell hooks, which would o= therwise run on ;; all the repo's files. ;; Also skip the pre-commit-setup hook, which isn't rele= vant since ;; we're not using the pre-commit tool. (when (or (equal? (assoc-ref hook 'stages) '(commit-msg)) (string=3D? (assoc-ref hook 'id) "pre-commit-s= etup")) (return-hook previous)) (message "Running hook ~a ..." (or (assoc-ref hook 'name) (assoc-ref hook 'id))) (let* ((version-required (assoc-ref hook 'version-requir= ed)) (current-version (and version-required ((assoc-re= f hook 'get-version))))) (unless (or (not version-required) (string=3D? current-version version-requir= ed)) (error-or-warning "We need ~a version ~a, but we hav= e ~a." (assoc-ref hook 'id) version-requi= red current-version) (unless *ignore-tool-checks?* (return-hook 3)))) (let* ((entry (assoc-ref hook 'entry)) (types-or (assoc-ref hook 'types-or)) (files (assoc-ref hook 'files)) (files-regexp (and files (make-regexp files))) (exclude (assoc-ref hook 'exclude)) (exclude-regexp (and exclude (make-regexp exclude= ))) (args (or (assoc-ref hook 'args) '())) (guix-packages (assoc-ref hook 'guix-packages)) (result (run-cmd (append (list entry) args (filter-repo-files #:types-or ty= pes-or #:files files= -regexp #:exclude exc= lude-regexp)) #:guix-packages guix-packages))) (max previous (if result 0 1))))) 0 hooks)))))) --=-=-=--