FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-111062: Reusable Ubuntu build that supports free-threaded mode as the conditional CI by NCLI · Pull Request #111452 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-111062: Reusable Ubuntu build that supports free-threaded mode as the conditional CI - #111452

Merged
hugovk merged 29 commits into
python:mainfrom
NCLI:reusable_ubuntu_build_nogil
Oct 30, 2023
ConversationCommits (29)ChecksFiles changed
Merged

gh-111062: Reusable Ubuntu build that supports free-threaded mode as the conditional CI#111452
hugovk merged 29 commits into
python:mainfrom
NCLI:reusable_ubuntu_build_nogil

Conversation

NCLI commented Oct 29, 2023 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

bedevere-app Bot commented Oct 29, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

bedevere-app Bot commented Oct 29, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

bedevere-app Bot commented Oct 29, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

bedevere-app Bot commented Oct 29, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

bedevere-app Bot commented Oct 29, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

corona10 reviewed Oct 29, 2023
View reviewed changes
Comment thread .github/workflows/build.yml Outdated
corona10 reviewed Oct 29, 2023
View reviewed changes
Comment thread .github/workflows/build.yml Outdated
build_ubuntu_free_threaded:
name: 'Ubuntu (free-threaded)'
needs: check_source
if: needs.check_source.outputs.run_tests == 'true'

corona10 Oct 29, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We should check that the label is attached or not.

corona10 requested changes Oct 29, 2023
View reviewed changes

corona10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please skip the test if the label is not attached
https://github.com/python/cpython/pull/111452/files#r1375408558

bedevere-app Bot commented Oct 29, 2023

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

corona10 reviewed Oct 29, 2023
View reviewed changes
Comment thread .github/workflows/build.yml Outdated
corona10 approved these changes Oct 29, 2023
View reviewed changes

corona10 left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Now it looks good to me
@hugovk @ezio-melotti Can you please take a look?

hugovk commented Oct 29, 2023

Copy link
Copy Markdown
Member

I'll add the https://github.com/python/cpython/labels/topic-free-threaded label to check if it triggers.

hugovk commented Oct 29, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

The label would need to have been added before the workflow runs, which means you need to add it when drafting the PR or do it very quickly.

Or we need to add triggers similar to this:

cpython/.github/workflows/require-pr-label.yml

Lines 3 to 5 in fa35b9e

on:
pull_request:
types: [opened, reopened, labeled, unlabeled, synchronize]

But that adds extra complexity because we don't want to re-trigger all the other CI jobs.

More to the point: do we really need to limit this via a label?

Should we always run it?

After all, we want to make sure there are no regressions for both the GIL and free-threaded builds, whether or not someone considers their PR to somehow related free-threaded.

corona10 commented Oct 29, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

The label would need to have been added before the workflow runs, which means you need to add it when drafting the PR > or do it very quickly.

Or we need to add triggers similar to this:
More to the point: do we really need to limit this via a label?
Should we always run it?

I know this issue, but topiuc-free-thread works are not a big portion of over all PRs so I think that it can be taken into account anyway at this moment. In the near future it can be run anyway once the free-thread version becomes stable but looks like not this moment. :)

And as I commented before, I don't want that free-threaded work to make other people's work stop.

corona10 commented Oct 29, 2023

Copy link
Copy Markdown
Member

The label would need to have been added before the workflow runs, which means you need to add it when drafting the PR or do it very quickly.

And it's intended design at this moment, or we need to improve it at later

corona10 closed this Oct 29, 2023
corona10 reopened this Oct 29, 2023

corona10 commented Oct 29, 2023

Copy link
Copy Markdown
Member

And simply re-open the PR will works if the person miss the timing of the labeling

This was referenced Oct 30, 2023
Closed
Closed
hugovk approved these changes Oct 30, 2023
View reviewed changes

hugovk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks, let's merge this.

We can remove the label-handling later when we're all ready for it.

Let's also consider the YML file names later, once the Windows one is also done.

And at the same time we can see about naming the "build_x" bit (the third part in "Tests / OS / build_x") in a nicer and more consistent way:

PS We'll need to backport all of these PRs to 3.11/3.12 -- but obviously not the free threaded part. And we can take care of that later too.

hugovk merged commit c19561b into python:main Oct 30, 2023
hugovk changed the title gh-111062: Reusable ubuntu build that supports free-threaded mode as the conditional CI gh-111062: Reusable Ubuntu build that supports free-threaded mode as the conditional CI Oct 30, 2023

NCLI commented Oct 30, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for the help @corona10 :-)

FullteaR pushed a commit to FullteaR/cpython that referenced this pull request Nov 3, 2023
pythongh-111062: Reusable ubuntu build that supports free-threaded mo…
b4d3b36 
…de as the conditional CI (python#111452)

Co-authored-by: Donghee Na <donghee.na92@gmail.com>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
pythongh-111062: Reusable ubuntu build that supports free-threaded mo…
29a07a6 
…de as the conditional CI (python#111452)

Co-authored-by: Donghee Na <donghee.na92@gmail.com>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
pythongh-111062: Reusable ubuntu build that supports free-threaded mo…
998bc16 
…de as the conditional CI (python#111452)

Co-authored-by: Donghee Na <donghee.na92@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

hugovk hugovk approved these changes
corona10 corona10 approved these changes
ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner

Assignees

hugovk

Labels

skip news topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL