GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-109408: Add a ``make lint`` target by AA-Turner · Pull Request #122333 · python/cpython

Conversation

@AA-Turner

@AA-Turner AA-Turner commented Jul 26, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

I am almost certainly not the best person to do this, given I use Windows and I am not confident in testing changes to the Makefile.

However, this is a sketch of a solution to adding a make lint target, as discussed in on Discourse and related to #109895.

Please feel free to push to this branch if you have improvements / spot glaring errors.

A

  • Issue: Move Azure Pipelines CI to GitHub Actions #109408

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Eclips4 and erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@AA-Turner
Add a make lint target
71740aa
@AA-Turner AA-Turner added the skip news label Jul 26, 2024
@AA-Turner
AA-Turner requested review from AlexWaygood, ezio-melotti, hugovk and merwok July 26, 2024 23:02
@AA-Turner
AA-Turner requested a review from erlend-aasland as a code owner July 26, 2024 23:02
@bedevere-app bedevere-app Bot mentioned this pull request Jul 26, 2024
Move Azure Pipelines CI to GitHub Actions #109408
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label Jul 26, 2024
AA-Turner added 2 commits July 27, 2024 00:03
@AA-Turner
tabs
edb0227
@AA-Turner
tabs (again)
9b7f8e5
@AA-Turner AA-Turner mentioned this pull request Jul 26, 2024
gh-109408: Stop running patchcheck in CI #109895
Merged
@encukou

encukou commented Jul 27, 2024

Copy link
Copy Markdown
Member

Who vouches for the security of the software that make lint will install on my machine?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented Jul 27, 2024

Copy link
Copy Markdown
Contributor

Who vouches for the security of the software that make lint will install on my machine?

It is installed in a virtual environment. We already require folks to install third-party dependencies for extension module, not to mention various compiler toolchains for building.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Jul 27, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Yup. For pip & setuptools it's PyPA; for compilers it's (for me) the distro. Here it's harder to keep up.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AA-Turner

AA-Turner commented Jul 27, 2024

Copy link
Copy Markdown
Member Author

Repeating Greg's comment from the Discourse thread re naming:

What I could potentially imagine as nicer than make patchcheck or make lint is actually make push… that runs meaningful checks that must pass prior to doing a git push of my branch to a git repo name of my choosing.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AA-Turner

AA-Turner commented Jul 27, 2024

Copy link
Copy Markdown
Member Author

Who vouches for the security of the software that make lint will install on my machine?
...
For pip & setuptools it's PyPA; for compilers it's (for me) the distro. Here it's harder to keep up.

I think there are a couple of approaches we could take here. One is to use the pre-commit binary from the OS if it is installed, which allows you to trust your OS distribution (e.g. fedora, debian, gentoo). Another is to pin the version of pre-commit, and require a committer to update the pin. A third is to vendor pre-commit within CPython's source tree.

On a more philosophical level, I'm not sure the level of assurance we can provide -- to take the PyPA per your example, they provide a high level governance framework, rather than managing individual projects. A good example might be the documentation, where we pull in Sphinx, sphinxext-opengraph, sphinx-notfound-page, and many dependencies. There is no CPython assurance of any of these projects (beyond perhaps 'blurb' and 'python-docs-theme').

My proposal would be to use an OS-provided binary if it exists, which would also benefit in avoiding a venv if the user has pre-commit globally installed. Would this alleviate your concerns?

A

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk

hugovk commented Jul 27, 2024

Copy link
Copy Markdown
Member

Also it is not required to install it locally, it runs on CI in any case.

erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented Jul 27, 2024

Copy link
Copy Markdown
Contributor

Also it is not required to install it locally, it runs on CI in any case.

Yep. You don't need to install this, @encukou; it's optional :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Jul 29, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

use the pre-commit binary from the OS if it is installed

The pre-commit binary is one thing. Another is the individual linters. They are installed from mutable tags on GitHub, and AFAIK, we don't have a policy for adding new ones or updating existing ones.

to take the PyPA per your example, they provide a high level governance framework, rather than managing individual projects.

Right, I said PyPA for short. The long story is that pip and setuptools are maintained by specific PyPA members, with (usually) close ties to the core dev team.
Still, it's something to keep in mind as a potential attack vector for getting into core devs' (and contributors') systems.

documentation, where we pull in Sphinx, sphinxext-opengraph, sphinx-notfound-page, and many dependencies

Yes. That's another place where I think we could improve handling the security implications.

You don't need to install this, @encukou; it's optional :)

Sure, I'm not really planning to use this.
But, while we're all used to pip downloading code from the 'net, having a make command do that is much more surprising -- at least to me.
[edited to add:] It feels like CPython should vouch for the security of its make targets.

merwok reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@StanFromIreland StanFromIreland added the infra CI, GitHub Actions, buildbots, Dependabot, etc. label Aug 20, 2025
@hugovk
hugovk removed their request for review April 9, 2026 21:26
@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merwok merwok Awaiting requested review from merwok
@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti
@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting core review infra CI, GitHub Actions, buildbots, Dependabot, etc. skip news stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@AA-Turner @encukou @erlend-aasland @hugovk @StanFromIreland
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.