Old pre-commit stage names are not transformed correctly into new names

lorenzwalthert commented 1 month ago

my .pre-commit-config.yaml from https://github.com/lorenzwalthert/precommit looked like this:

# All available hooks: https://pre-commit.com/hooks.html
# R specific hooks: https://github.com/lorenzwalthert/precommit
repos:
-   repo: https://github.com/lorenzwalthert/precommit
    rev: v0.4.3.9001
    hooks: 
    -   id: style-files
        args: [--style_pkg=styler, --style_fun=tidyverse_style, --cache-root=styler-perm]
        exclude: > 
          (?x)^(
          tests/testthat/in/.*|
          renv/.*
          )$
    -   id: roxygenize
        additional_dependencies:
        -    cli
        -    fs
        -    here
        -    magrittr
        -    purrr
        -    R.cache
        -    rlang
        -    rprojroot
        -    rstudioapi
        -    withr
        -    yaml
        -    r-lib/pkgapi
    # codemeta must be above use-tidy-description when both are used
    # -   id: codemeta-description-updated
    -   id: use-tidy-description
    -   id: spell-check
        exclude: >
          (?x)^(
          .*\.[rR]|
          .*\.feather|
          .*\.jpeg|
          .*\.pdf|
          .*\.png|
          .*\.py|
          .*\.RData|
          .*\.rds|
          .*\.Rds|
          .*\.Rproj|
          .*\.sh|
          (.*/|)\.gitignore|
          (.*/|)\.pre-commit-.*|
          (.*/|)\.Rbuildignore|
          (.*/|)\.Renviron|
          (.*/|)\.Rprofile|
          (.*/|)\.travis\.yml|
          (.*/|)appveyor\.yml|
          (.*/|)NAMESPACE|
          (.*/|)renv/settings\.dcf|
          (.*/|)renv\.lock|
          (.*/|)WORDLIST|
          \.github/workflows/.*|
          data/.*|
          inst/hooks/.*|
          inst/pre-commit-.*|
          inst/usethis-legacy-hook|
          LICENSE|
          renv/.*|
          revdep/.*|
          tests/testthat/in/.*|
          )$
    -   id: readme-rmd-rendered
    -   id: parsable-R
        exclude: > 
          (?x)^(
          tests/testthat/in/.*|
          )$
    -   id: no-browser-statement
        exclude: > 
          (?x)^(
          tests/testthat/in/.*|
          )$
    -   id: no-debug-statement
        exclude: >
          (?x)^(
          tests/testthat/in/.*|
          )$
    -   id: deps-in-desc
        exclude: > 
          (?x)^(
          inst/hooks/exported/pkgdown.R|
          tests/testthat/in/.*|
          inst/update-renv-prepare.R|
          inst/update-ppm-url.R|
          inst/update-dependency-graph-existing-packages\.R|
          inst/update-existing-hook-dependencies\.R|
          renv/activate.R|
          vignettes/FAQ\.Rmd|
          )$
    -   id: pkgdown
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks: 
    -   id: check-added-large-files
    -   id: end-of-file-fixer
        exclude: '\.Rd' # sometimes roxygen fails to generate EOF blank line.
    -   id: file-contents-sorter
        files: '^\.Rbuildignore$'
-   repo: https://github.com/pre-commit-ci/pre-commit-ci-config
    rev: v1.6.1
    hooks:
    # Only required when https://pre-commit.ci is used for config validation
    -   id: check-pre-commit-ci-config
-   repo: https://github.com/lorenzwalthert/gitignore-tidy
    rev: 0.1.2
    hooks: 
    -   id: tidy-gitignore
-   repo: local
    hooks:
    -   id: consistent-release-tag
        name: consistent-release-tag
        entry: Rscript inst/hooks/local/consistent-release-tag.R
        language: r
        additional_dependencies:
        -    docopt
        -    fs
        -    yaml
        -    purrr
        -    glue
        -    rlang
        -    git2r
        -    desc
        -    lorenzwalthert/precommit
        stages: [commit, push]
    -   id: hooks-config-to-inst
        name: hooks-config-to-inst
        entry: Rscript inst/hooks/local/hooks-config-to-inst.R
        language: r
        stages: [commit, push]
        additional_dependencies:
        -    fs
        require_serial: True
    -   id: spell-check-exclude-identical
        name: spell-check-exclude-identical
        entry: Rscript inst/hooks/local/spell-check-exclude-identical.R
        language: r
        stages: [commit, push]
        additional_dependencies:
        -    magrittr
        -    purrr
        -    yaml
        -    here
        -    rlang
        require_serial: True
    -   id: forbid-to-commit
        name: Don't commit common R artifacts
        entry: Cannot commit .Rhistory, .Rdata, .csv and similar.
        language: fail
        files: '\.(Rhistory|csv|RData|Rds|rds)$'
        # `exclude: <regex>` to allow committing specific files.
    -   id: spell-check-ordered-exclude
        name: Ordered regex pattern for spell-check exclusion
        description: Ensure alphabetical order in `exclude:` key of spell check.
        entry: Rscript inst/hooks/local/spell-check-ordered-exclude.R
        language: r
        files: '^(.*/|)\.?pre-commit-config.*\.yaml$'
        additional_dependencies:
        -    magrittr
        -    yaml
        -    purrr
        -    rlang

default_stages: ["commit"]

ci:
    skip: [consistent-release-tag, spell-check-ordered-exclude, pkgdown]
    autoupdate_schedule: monthly

and pre-commit.ci turned it into

# All available hooks: https://pre-commit.com/hooks.html
# R specific hooks: https://github.com/lorenzwalthert/precommit
repos:
-   repo: https://github.com/lorenzwalthert/precommit
    rev: v0.4.3.9001
    hooks: 
    -   id: style-files
        args: [--style_pkg=styler, --style_fun=tidyverse_style, --cache-root=styler-perm]
        exclude: > 
          (?x)^(
          tests/testthat/in/.*|
          renv/.*
          )$
    -   id: roxygenize
        additional_dependencies:
        -    cli
        -    fs
        -    here
        -    magrittr
        -    purrr
        -    R.cache
        -    rlang
        -    rprojroot
        -    rstudioapi
        -    withr
        -    yaml
        -    r-lib/pkgapi
    # codemeta must be above use-tidy-description when both are used
    # -   id: codemeta-description-updated
    -   id: use-tidy-description
    -   id: spell-check
        exclude: >
          (?x)^(
          .*\.[rR]|
          .*\.feather|
          .*\.jpeg|
          .*\.pdf|
          .*\.png|
          .*\.py|
          .*\.RData|
          .*\.rds|
          .*\.Rds|
          .*\.Rproj|
          .*\.sh|
          (.*/|)\.gitignore|
          (.*/|)\.pre-commit-.*|
          (.*/|)\.Rbuildignore|
          (.*/|)\.Renviron|
          (.*/|)\.Rprofile|
          (.*/|)\.travis\.yml|
          (.*/|)appveyor\.yml|
          (.*/|)NAMESPACE|
          (.*/|)renv/settings\.dcf|
          (.*/|)renv\.lock|
          (.*/|)WORDLIST|
          \.github/workflows/.*|
          data/.*|
          inst/hooks/.*|
          inst/pre-commit-.*|
          inst/usethis-legacy-hook|
          LICENSE|
          renv/.*|
          revdep/.*|
          tests/testthat/in/.*|
          )$
    -   id: readme-rmd-rendered
    -   id: parsable-R
        exclude: > 
          (?x)^(
          tests/testthat/in/.*|
          )$
    -   id: no-browser-statement
        exclude: > 
          (?x)^(
          tests/testthat/in/.*|
          )$
    -   id: no-debug-statement
        exclude: >
          (?x)^(
          tests/testthat/in/.*|
          )$
    -   id: deps-in-desc
        exclude: > 
          (?x)^(
          inst/hooks/exported/pkgdown.R|
          tests/testthat/in/.*|
          inst/update-renv-prepare.R|
          inst/update-ppm-url.R|
          inst/update-dependency-graph-existing-packages\.R|
          inst/update-existing-hook-dependencies\.R|
          renv/activate.R|
          vignettes/FAQ\.Rmd|
          )$
    -   id: pkgdown
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v5.0.0
    hooks: 
    -   id: check-added-large-files
    -   id: end-of-file-fixer
        exclude: '\.Rd' # sometimes roxygen fails to generate EOF blank line.
    -   id: file-contents-sorter
        files: '^\.Rbuildignore$'
-   repo: https://github.com/pre-commit-ci/pre-commit-ci-config
    rev: v1.6.1
    hooks:
    # Only required when https://pre-commit.ci is used for config validation
    -   id: check-pre-commit-ci-config
-   repo: https://github.com/lorenzwalthert/gitignore-tidy
    rev: 0.1.2
    hooks: 
    -   id: tidy-gitignore
-   repo: local
    hooks:
    -   id: consistent-release-tag
        name: consistent-release-tag
        entry: Rscript inst/hooks/local/consistent-release-tag.R
        language: r
        additional_dependencies:
        -    docopt
        -    fs
        -    yaml
        -    purrr
        -    glue
        -    rlang
        -    git2r
        -    desc
        -    lorenzwalthert/precommit
        stages: [Nonepre-commitNone, Nonepre-pushNone]
    -   id: hooks-config-to-inst
        name: hooks-config-to-inst
        entry: Rscript inst/hooks/local/hooks-config-to-inst.R
        language: r
        stages: [Nonepre-commitNone, Nonepre-pushNone]
        additional_dependencies:
        -    fs
        require_serial: True
    -   id: spell-check-exclude-identical
        name: spell-check-exclude-identical
        entry: Rscript inst/hooks/local/spell-check-exclude-identical.R
        language: r
        stages: [Nonepre-commitNone, Nonepre-pushNone]
        additional_dependencies:
        -    magrittr
        -    purrr
        -    yaml
        -    here
        -    rlang
        require_serial: True
    -   id: forbid-to-commit
        name: Don't commit common R artifacts
        entry: Cannot commit .Rhistory, .Rdata, .csv and similar.
        language: fail
        files: '\.(Rhistory|csv|RData|Rds|rds)$'
        # `exclude: <regex>` to allow committing specific files.
    -   id: spell-check-ordered-exclude
        name: Ordered regex pattern for spell-check exclusion
        description: Ensure alphabetical order in `exclude:` key of spell check.
        entry: Rscript inst/hooks/local/spell-check-ordered-exclude.R
        language: r
        files: '^(.*/|)\.?pre-commit-config.*\.yaml$'
        additional_dependencies:
        -    magrittr
        -    yaml
        -    purrr
        -    rlang

default_stages: ["pre-commit"]

ci:
    skip: [consistent-release-tag, spell-check-ordered-exclude, pkgdown]
    autoupdate_schedule: monthly

and then failed:

build attempt 1...
    => error
    An error has occurred: InvalidConfigError: 
    ==> File .pre-commit-config.yaml
    ==> At Config()
    ==> At key: repos
    ==> At Repository(repo='local')
    ==> At key: hooks
    ==> At Hook(id='consistent-release-tag')
    ==> At index 0
    =====> Expected one of commit-msg, manual, post-checkout, post-commit, post-merge, post-rewrite, pre-commit, pre-merge-commit, pre-push, pre-rebase, prepare-commit-msg but got: 'Nonepre-commitNone'
    Check the log at /pc/pre-commit.log
build attempt 2...
    => error
    An error has occurred: InvalidConfigError: 
    ==> File .pre-commit-config.yaml
    ==> At Config()
    ==> At key: repos
    ==> At Repository(repo='local')
    ==> At key: hooks
    ==> At Hook(id='consistent-release-tag')
    ==> At index 0
    =====> Expected one of commit-msg, manual, post-checkout, post-commit, post-merge, post-rewrite, pre-commit, pre-merge-commit, pre-push, pre-rebase, prepare-commit-msg but got: 'Nonepre-commitNone'
    Check the log at /pc/pre-commit.log

asottile commented 1 month ago

ah this is probably a bug with pre-commit itself -- surprising that I didn't hit this with my testing (the token has None as it's quote instead of strings). will try and get a fix for that!

lorenzwalthert commented 1 month ago

Thanks @asottile, and sorry my reprex is a bit convoluted but I am sure you can derive a more clean one if needed.

asottile commented 1 month ago

nah this was perfect -- thank you for being thorough in your reproduction.

this will get fixed by https://github.com/pre-commit/pre-commit/pull/3324 (and the equivalent in the autoupdater service) so you should see the correct values on next week's autoupdate (you can either choose to fix the PR manually, or close it and it'll get recreated next week without the bug)

sorry for the inconvenience! yet another annoying quirk with pyyaml :)

lorenzwalthert commented 1 month ago

Great. Thanks so much.

pre-commit-ci / issues

Old pre-commit stage names are not transformed correctly into new names #234