Skip to content

ci(check_ecosystem): add PyPa/build#3569

Merged
charliermarsh merged 3 commits intoastral-sh:mainfrom
henryiii:henryiii/ci/build
Mar 18, 2023
Merged

ci(check_ecosystem): add PyPa/build#3569
charliermarsh merged 3 commits intoastral-sh:mainfrom
henryiii:henryiii/ci/build

Conversation

@henryiii
Copy link
Contributor

@henryiii henryiii commented Mar 17, 2023

Adding PyPA/build.

@henryiii
ci(check_ecosystem): add build
2c35c0d 
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2023 •
edited
Loading

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     13.8±0.06ms     3.0 MB/sec    1.00     13.8±0.07ms     3.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.6±0.01ms     4.6 MB/sec    1.00      3.6±0.01ms     4.6 MB/sec
linter/all-rules/numpy/globals.py          1.00    505.9±1.82µs     5.8 MB/sec    1.00    505.5±1.15µs     5.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.1±0.02ms     4.2 MB/sec    1.00      6.1±0.02ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.00      7.5±0.02ms     5.4 MB/sec    1.00      7.5±0.02ms     5.4 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1688.5±4.52µs     9.9 MB/sec    1.00   1686.5±4.27µs     9.9 MB/sec
linter/default-rules/numpy/globals.py      1.00    192.8±0.61µs    15.3 MB/sec    1.00    192.5±0.44µs    15.3 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.5±0.01ms     7.3 MB/sec    1.00      3.5±0.02ms     7.3 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.01     15.9±0.27ms     2.6 MB/sec    1.00     15.8±0.15ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      4.2±0.06ms     3.9 MB/sec    1.00      4.2±0.05ms     4.0 MB/sec
linter/all-rules/numpy/globals.py          1.00    544.0±8.03µs     5.4 MB/sec    1.00    542.2±8.34µs     5.4 MB/sec
linter/all-rules/pydantic/types.py         1.01      6.9±0.10ms     3.7 MB/sec    1.00      6.9±0.08ms     3.7 MB/sec
linter/default-rules/large/dataset.py      1.01      8.8±0.09ms     4.6 MB/sec    1.00      8.7±0.13ms     4.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1921.3±20.96µs     8.7 MB/sec    1.00  1907.8±21.42µs     8.7 MB/sec
linter/default-rules/numpy/globals.py      1.00    211.2±2.91µs    14.0 MB/sec    1.02    214.9±5.95µs    13.7 MB/sec
linter/default-rules/pydantic/types.py     1.00      4.1±0.05ms     6.3 MB/sec    1.01      4.1±0.07ms     6.2 MB/sec

@charliermarsh
Copy link
Member

charliermarsh commented Mar 17, 2023

@henryiii - Any ideas on how to resolve the above?

@henryiii
Copy link
Contributor Author

henryiii commented Mar 17, 2023

It’s reading the pyproject.toml’s in test, I guess? Those need to be ignored. There are examples of broken toml files. Running it normally doesn’t have this issue.

@charliermarsh
Copy link
Member

charliermarsh commented Mar 17, 2023

I do get it when running locally:

❯ ruff .
error: TOML parse error at line 2, column 19
  |
2 | requires = ['bad' 'syntax']
  |                   ^
invalid array
expected `]

Does the typical invocation run over src specifically, or something like that?

@henryiii
Copy link
Contributor Author

henryiii commented Mar 17, 2023 •
edited
Loading

Typical invocation is via pre-commit, https://github.com/pypa/build/blob/e496fe6342f0e4729b69d0ea93eef529f27875a6/.pre-commit-config.yaml#L41. It probably passes all Python files explicitly. Maybe it doesn’t pick up nested pyproject.toml files in that case?

@henryiii
Copy link
Contributor Author

henryiii commented Mar 17, 2023

Will it ignore them if I exclude tests/**/pyproject.toml?

@MichaReiser MichaReiser changed the title ci(check_ecosystem): add build ci(check_ecosystem): add PyPa/build Mar 17, 2023
@henryiii
Copy link
Contributor Author

henryiii commented Mar 17, 2023 

exclude = ["tests/packages/test-bad-syntax/pyproject.toml"]

Doesn't seem to get ruff . to ignore trying to read this pyproject.toml file.

@charliermarsh
Copy link
Member

charliermarsh commented Mar 17, 2023

I'll take a look, we need to exclude the entire directory so that it doesn't try to find the pyproject.toml file for any of the surfaced files.

@charliermarsh
Copy link
Member

charliermarsh commented Mar 17, 2023

Omitting test entirely works, but that's not what you want. Omitting just that directory doesn't seem to work. I'll take a look, we might need to fix something in Ruff.

@henryiii
Copy link
Contributor Author

henryiii commented Mar 17, 2023

Yeah, I tried omitting the directory and the file. Don't want to omit test. :)

@charliermarsh
Copy link
Member

charliermarsh commented Mar 17, 2023

Haha yeah I figured :)

We have to load configuration files as we traverse, because the set of exclusions can change as we go deeper into the filesystem and find the relevant configuration files for each directory. So I'm guessing we're loading that config too eagerly somewhere despite the exclusion.

@charliermarsh charliermarsh mentioned this pull request Mar 17, 2023
Merged
@MichaReiser MichaReiser mentioned this pull request Mar 18, 2023
Merged
@henryiii
Copy link
Contributor Author

henryiii commented Mar 18, 2023

I expect the exclusion still needs to be added to build. I can do that in a bit.

@charliermarsh
Copy link
Member

charliermarsh commented Mar 18, 2023

🤦 Right right, sorry. A release isn't necessary, but the exclusion does need to be added.

@henryiii henryiii mentioned this pull request Mar 18, 2023
Merged
@henryiii
Copy link
Contributor Author

henryiii commented Mar 18, 2023

Why is it passing, actually?

@charliermarsh
Copy link
Member

charliermarsh commented Mar 18, 2023

The ecosystem check doesn't fail CI, but the error is being reported in the comment, I think:

Screen Shot 2023-03-18 at 4 08 32 PM

@charliermarsh charliermarsh merged commit 4bdb2dd into astral-sh:main Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@henryiii @charliermarsh