[PATCH] [pre-commit] Add shellcheck

[PATCH] [pre-commit] Add shellcheck

[Tom de Vries]

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
---
 .pre-commit-config.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 141451501ed..630f85b8127 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -129,3 +129,11 @@ repos:
       language: unsupported_script
       entry: gdb/contrib/check-file-mode.sh
       files: '^(gdb|gdbserver|gdbsupport)/.*$'
+    - id: &id5 shellcheck
+      name: *id5
+      language: python
+      additional_dependencies: ['pureshellcheck==0.2.2']
+      entry: pureshellcheck
+      files: '^(gdb|gdbsupport|gdbserver)/'
+      types: ['shell']
+      exclude: '^(gdb/config/djgpp/(djcheck.sh|djconfig.sh)|gdb/contrib/ari/(create-web-ari-in-src.sh|gdb_ari.sh|update-web-ari.sh)|gdb/contrib/(cc-with-tweaks.sh|expect-read1.sh|gdb-add-index.sh|license-check-new-files.sh)|gdb/gdb_buildall.sh|gdb/gdb_mbuild.sh|gdb/regformats/regdat.sh|gdb/syscalls/update-linux-from-src.sh|gdb/testsuite/make-check-all.sh|gdb/gcore-1.in|gdb/po/gdbtext|gdb/testsuite/lib/pdtrace.in)$|/configure$'

base-commit: 478b1846cdb82b776c8dcbec20ecd198297cd8ff
-- 
2.51.0

Re: [PATCH] [pre-commit] Add shellcheck

[Simon Marchi]

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.

Assuming you gave a quick look at the code of this project and judge
that it's fine, how can we ensure that whatever pre-commit pulls is what
you reviewed?  We use a git tag, but is that sufficient?  Could a
(malicious or compromised) project publish a tag, and then replaced that
tag with something else later?

An alternative could be to point to a specific commit hash.  A more
radical alternative would be to vendor (put in our repo) the code of the
hooks we use.

I'd like to hear your thoughts on this.

Simon

Re: [PATCH] [pre-commit] Add shellcheck

[Tom de Vries]

On 6/18/26 7:05 PM, Simon Marchi wrote:
> 
> 
> 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.
> 
> Assuming you gave a quick look at the code of this project and judge
> that it's fine,

Yeah, I did, that is, I cloned the github repository, checked it out at 
main/v0.2.2, and asked claude code to review it on safety aspects.

> how can we ensure that whatever pre-commit pulls is what
> you reviewed?

The mechanism to get the tool proposed in this patch is via pip.  The 
pypi index mentions the github repo as source, so I'm relying on that.

> We use a git tag, but is that sufficient?  Could a
> (malicious or compromised) project publish a tag, and then replaced that
> tag with something else later?

I suppose it's possible.

> An alternative could be to point to a specific commit hash.  A more
> radical alternative would be to vendor (put in our repo) the code of the
> hooks we use.
> 

Yeah, that is safer.  It would mean though using github as the source. 
But the repo as is doesn't have pre-commit hooks.  I worked around that 
before by using my own github account (see commit 7f6c7a5bb37 
("[pre-commit] Add tclint hook")).  I'll submit a v2 shortly that uses 
this approach.

FWIW, I'm open to any other solutions.

Thanks,
- Tom

> I'd like to hear your thoughts on this.
> 
> Simon

Re: [PATCH] [pre-commit] Add shellcheck

[Simon Marchi]

On 2026-06-19 07:19, Tom de Vries wrote:
> On 6/18/26 7:05 PM, Simon Marchi wrote:
>>
>>
>> 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.
>>
>> Assuming you gave a quick look at the code of this project and judge
>> that it's fine,
> 
> Yeah, I did, that is, I cloned the github repository, checked it out at main/v0.2.2, and asked claude code to review it on safety aspects.

Cool, thanks.

>> how can we ensure that whatever pre-commit pulls is what
>> you reviewed?
> 
> The mechanism to get the tool proposed in this patch is via pip.  The pypi index mentions the github repo as source, so I'm relying on that.

Ok I didn't get this at first.  You specify the tool (and exact version)
using additional_dependencies, that downloads it in the venv, and then
it runs "pureshellcheck" since it's specified as the entry point.

I previously thought it would get it from the github repo directly, but
no (the github repo URL isn't even mentioned anywhere in the hook).

Getting a specific version from pypi seems safe-ish, in that it's not
possible (from what I've read) to swap a file with another, keeping the
same name.  Although I read that it might be possible for someone to
upload a source package at first, and then only later upload a binary
one for the same version (which could differ and contain something
nasty).  pip would then prefer the binary one as soon as it's uploaded.

From what I understand, with hooks like flake8, pre-commit clones the
specified ref, then does "pip install" in it to install it in the venv.
So we use the flake8 code as specified by that ref, but its dependencies
are still downloaded from pypi, we can't get around that.

>> We use a git tag, but is that sufficient?  Could a
>> (malicious or compromised) project publish a tag, and then replaced that
>> tag with something else later?
> 
> I suppose it's possible.

I found out about

  $ pre-commit autoupdate --freeze

it transforms:

    rev: 26.5.1

into

    rev: 4160603246a6b365d4a2af661c6d71b0a0f50478  # frozen: 26.5.1

It won't solve all the integrity concerns, but we might as well use
that, I don't see a downside.

>> An alternative could be to point to a specific commit hash.  A more
>> radical alternative would be to vendor (put in our repo) the code of the
>> hooks we use.
>>
> 
> Yeah, that is safer.  It would mean though using github as the source. But the repo as is doesn't have pre-commit hooks.  I worked around that before by using my own github account (see commit 7f6c7a5bb37 ("[pre-commit] Add tclint hook")).  I'll submit a v2 shortly that uses this approach.
> 
> FWIW, I'm open to any other solutions.

Could you maybe submit a .pre-commit-hooks.yaml to that project?  And
then we can point to a specific tag/hash of that repo.  If we don't hear
anything from them after a few weeks then we could consider your fork,
but if we can have the hooks upstream from the start it's less
maintenance.

Simon

Re: [PATCH] [pre-commit] Add shellcheck

[Thiago Jung Bauermann]

[-- Attachment #1: Type: text/plain, Size: 2354 bytes --]

Simon Marchi <simark@simark.ca> 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)


[-- Attachment #2: run-gdb-precommit-hooks --]
[-- Type: text/scheme, Size: 17932 bytes --]

#!/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 ": " message "\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 arguments))

(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 — inside a container if CONTAINER? is #t.
   If OUTPUT is a string, it's the path of a file to which output will be redirected.
   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=" (get-gdb-repo-path))
                             ;; 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 output-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 . ,(λ ()
                           ;; Example output:
                           ;; .black-real, 25.1.0 (compiled: no)
                           (list-ref (string-tokenize (run-cmd '("black" "--version")
                                                               #:guix-packages '("python-black")
                                                               #:output #t))
                                     1)))
         (entry . "black")
         (guix-packages . ("python-black")))
       `((id . "flake8")
         (get-version . ,(λ ()
                           ;; Example output:
                           ;; 7.1.1 (mccabe: 0.7.0, pycodestyle: 2.12.1, pyflakes: 3.2.0) CPython 3.11.14 on Linux
                           (list-ref (string-tokenize (run-cmd '("flake8" "--version")
                                                               #:guix-packages '("python-flake8")
                                                               #:output #t))
                                     0)))
         (entry . "flake8")
         (guix-packages . ("python-flake8")))
       `((id . "isort")
         (get-version . ,(λ ()
                           ;; 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 . ,(λ ()
                           ;; Example output:
                           ;; 2.3.0
                           (string-append "v" (run-cmd '("codespell" "--version")
                                                       #:guix-packages '("python-codespell")
                                                       #:output #t))))
         (entry . "codespell")
         (guix-packages . ("python-codespell")))
       `((id . "tclint")
         (get-version . ,(λ ()
                           ;; FIXME: For some reason, my tclint Guix package 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/…-tclint-0.8.0
                           (string-append "v"
                                          (list-ref (string-tokenize
                                                     (run-cmd '("guix" "package" "-I" "tclint")
                                                              #:guix-packages '("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 (λ (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 (λ (_ 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 "default_stages"))))
    ;; Loop through the vector of repos, building the list of association lists on the way.
    (vector-fold (λ (_ result repo)
                   ;; Loop through the repo's vector of hooks, building the list of
                   ;; association lists on the way.
                   (vector-fold (λ (_ hooks-result hook)
                                  (let* ((id (assoc-ref hook "id"))
                                         (hook-info (get-hook-info id))
                                         (local? (string=? (assoc-ref repo "repo") "local"))
                                         (rev (assoc-ref repo "rev")))
                                    (if (and hooks-result (or hook-info local?))
                                        (let* ((get-version (assoc-ref hook-info 'get-version))
                                               (entry (or (assoc-ref hook "entry")
                                                          (assoc-ref hook-info 'entry)))
                                               (types-or (yaml-vector->symbols-list
                                                          (assoc-ref hook "types_or")))
                                               (files (assoc-ref hook "files"))
                                               (exclude (assoc-ref hook "exclude"))
                                               (info-args (or (assoc-ref hook-info 'args) '()))
                                               (hook-args (vector->list (or (assoc-ref hook "args")
                                                                            (make-vector 0))))
                                               (guix-packages (or (assoc-ref 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-required rev)
                                                         (cons 'get-version get-version)
                                                         (cons 'entry entry)
                                                         (cons 'types-or types-or)
                                                         (cons 'files files)
                                                         (cons 'args (append info-args hook-args))
                                                         (cons 'guix-packages guix-packages)
                                                         (cons 'stages stages)
                                                         (cons 'exclude exclude)))))
                                        (begin
                                          (unless hook-info
                                            (error-or-warning "Unknown hook ~a." id))
                                          (if *ignore-tool-checks?* hooks-result #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 guix command is
                                   ;; available.
                                   (run-cmd '("guix" "--version")
                                            #:output "/dev/null"
                                            #:container? #f)))

       ;; If Guix isn't available, fail unless "--no-container" has been given.
       (when (and container? (not *use-container?*))
         (error-message "Use of containers requested but Guix isn't available.")
         (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 (λ (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 otherwise run on
                   ;; all the repo's files.
                   ;; Also skip the pre-commit-setup hook, which isn't relevant since
                   ;; we're not using the pre-commit tool.
                   (when (or (equal? (assoc-ref hook 'stages) '(commit-msg))
                             (string=? (assoc-ref hook 'id) "pre-commit-setup"))
                     (return-hook previous))

                   (message "Running hook ~a ..." (or (assoc-ref hook 'name)
                                                      (assoc-ref hook 'id)))

                   (let* ((version-required (assoc-ref hook 'version-required))
                          (current-version (and version-required ((assoc-ref hook 'get-version)))))
                     (unless (or (not version-required)
                                 (string=? current-version version-required))
                       (error-or-warning "We need ~a version ~a, but we have ~a."
                                         (assoc-ref hook 'id) version-required 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 types-or
                                                              #:files files-regexp
                                                              #:exclude exclude-regexp))
                                   #:guix-packages guix-packages)))
                     (max previous (if result 0 1)))))
               0
               hooks))))))

Re: [PATCH] [pre-commit] Add shellcheck

[Tom de Vries]

On 6/19/26 7:56 PM, Simon Marchi wrote:
> 
> 
> On 2026-06-19 07:19, Tom de Vries wrote:
>> On 6/18/26 7:05 PM, Simon Marchi wrote:
>>>
>>>
>>> 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.
>>>
>>> Assuming you gave a quick look at the code of this project and judge
>>> that it's fine,
>>
>> Yeah, I did, that is, I cloned the github repository, checked it out at main/v0.2.2, and asked claude code to review it on safety aspects.
> 
> Cool, thanks.
> 
>>> how can we ensure that whatever pre-commit pulls is what
>>> you reviewed?
>>
>> The mechanism to get the tool proposed in this patch is via pip.  The pypi index mentions the github repo as source, so I'm relying on that.
> 
> Ok I didn't get this at first.  You specify the tool (and exact version)
> using additional_dependencies, that downloads it in the venv, and then
> it runs "pureshellcheck" since it's specified as the entry point.
> 
> I previously thought it would get it from the github repo directly, but
> no (the github repo URL isn't even mentioned anywhere in the hook).
> 
> Getting a specific version from pypi seems safe-ish, in that it's not
> possible (from what I've read) to swap a file with another, keeping the
> same name.  Although I read that it might be possible for someone to
> upload a source package at first, and then only later upload a binary
> one for the same version (which could differ and contain something
> nasty).  pip would then prefer the binary one as soon as it's uploaded.
> 
>  From what I understand, with hooks like flake8, pre-commit clones the
> specified ref, then does "pip install" in it to install it in the venv.
> So we use the flake8 code as specified by that ref, but its dependencies
> are still downloaded from pypi, we can't get around that.
> 
>>> We use a git tag, but is that sufficient?  Could a
>>> (malicious or compromised) project publish a tag, and then replaced that
>>> tag with something else later?
>>
>> I suppose it's possible.
> 
> I found out about
> 
>    $ pre-commit autoupdate --freeze
> 
> it transforms:
> 
>      rev: 26.5.1
> 
> into
> 
>      rev: 4160603246a6b365d4a2af661c6d71b0a0f50478  # frozen: 26.5.1
> 
> It won't solve all the integrity concerns, but we might as well use
> that, I don't see a downside.
>

Ack, that's what I proposed to use in v2.

Note that the --freeze only works until the next autoupdate without 
freeze.  So we'll likely need an autoupdate script that skips 
pureshellcheck, along the lines of:
...
$ cat contrib/pre-commit-autoupdate.sh
#!/bin/bash

repos=($(grep "\- repo: " .pre-commit-config.yaml \
	     | awk '{print $3}'))

for repo in "${repos[@]}"; do
     if [ "$repo" = local ]; then
	continue
     fi

     pre-commit autoupdate \
	       --repo "$repo"
done
...

>>> An alternative could be to point to a specific commit hash.  A more
>>> radical alternative would be to vendor (put in our repo) the code of the
>>> hooks we use.
>>>
>>
>> Yeah, that is safer.  It would mean though using github as the source. But the repo as is doesn't have pre-commit hooks.  I worked around that before by using my own github account (see commit 7f6c7a5bb37 ("[pre-commit] Add tclint hook")).  I'll submit a v2 shortly that uses this approach.
>>
>> FWIW, I'm open to any other solutions.
> 
> Could you maybe submit a .pre-commit-hooks.yaml to that project?  And
> then we can point to a specific tag/hash of that repo.  If we don't hear
> anything from them after a few weeks then we could consider your fork,
> but if we can have the hooks upstream from the start it's less
> maintenance.
> 

I've done that, and submitted fixes for two problems.

If that doesn't work out, perhaps some setup on our local sourceware 
forge owned by the gdb project rather than my personal github account 
would be better.  Not sure how to get that setup though, I haven't used 
it yet.

Thanks,
- Tom

> Simon

Re: [PATCH] [pre-commit] Add shellcheck

[Tom de Vries]

On 6/22/26 7:48 AM, Thiago Jung Bauermann wrote:
> Simon Marchi <simark@simark.ca> 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. :)
> 

Hi Thiago,

thanks for sharing your point of view and the script.

My take on this is as follows.

The benefit of git hooks is automation.

The benefit of using the pre-commit framework to manage those hooks is:
- version pinning, and
- seamless installation
in other words, making it easy that everybody does the same check.

That comes with the drawback of a download-heavy approach, but that's 
the trade-off.

Your approach has a different trade-off.

We could do something similar (no downloading) for shellcheck:
...
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 9910bbb82f2..92c56d95977 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -130,3 +130,10 @@ repos:
        language: unsupported_script
        entry: gdb/contrib/check-file-mode.sh
        files: '^(gdb|gdbserver|gdbsupport)/.*$'
+    - id: &id5 shellcheck
+      name: *id5
+      language: unsupported
+      entry: shellcheck
+      files: '^(gdb|gdbsupport|gdbserver)/'
+      types: ['shell']
+      exclude: 
'^(gdb/config/djgpp/(djcheck.sh|djconfig.sh)|gdb/contrib/ari/(create-web-ari-
in-src.sh|gdb_ari.sh|update-web-ari.sh)|gdb/contrib/(cc-with-tweaks.sh|gdb-add-index.sh)|gdb/gdb_builda
ll.sh|gdb/gdb_mbuild.sh|gdb/regformats/regdat.sh|gdb/po/gdbtext|gdb/testsuite/lib/pdtrace.in)$|/configu
re$'
...

This means there's no easy installation, people have to install it 
themselves.

There's also no version pinning.  We could add a script that does a 
version check though, and check for version 0.11.0.

My laptop runs Leap 16.0, which has shellcheck 0.10.0, so I would have 
to run pre-commit in a tumbleweed container.  Or build shellcheck from 
source and add it to my path.

But yeah, I'd prefer to not have to bother with that, and pureshellcheck 
does provide an easy solution.

Thanks,
- Tom