Skip to content

JIT README: How to install LLVM on Fedora Linux - #118983

Merged
brandtbucher merged 2 commits into
python:mainfrom
hroncok:patch-2
May 16, 2024
Merged

JIT README: How to install LLVM on Fedora Linux#118983
brandtbucher merged 2 commits into
python:mainfrom
hroncok:patch-2

Conversation

@hroncok

@hroncokhroncok commented May 13, 2024

Copy link
Copy Markdown
Contributor

@hroncok

hroncok commented May 13, 2024

Copy link
Copy Markdown
ContributorAuthor

As a side note, I've also inquired about having LLVM 18 available in Fedora Linux 39.

@Eclips4

Eclips4 commented May 13, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Miro, is this really should target 3.13 branch? I guess you have chose this one accidentally.
For me, it looks like it should target the current main branch (3.14), and it would be nice to backport it to 3.13, since JIT is available in 3.13.

@hroncok
hroncok changed the base branch from 3.13 to mainMay 13, 2024 10:32
@Eclips4

Eclips4 commented May 13, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Miro, is this really should target 3.13 branch? I guess you have chose this one accidentally. For me, it looks like it should target the current main branch (3.14), and it would be nice to backport it to 3.13, since JIT is available in 3.13.

Nah, I was doing this on my phone from GitHub web edit and I managed to click the edit button on the 3.13 branch, sorry about that.

Now I belive I fixed this, but at the same time, GitHub decided to spam everybody. I am so sorry 😢

No worries! Bad things happens sometimes with Github UI/UX :). I've dismissed requests for review from everyone except Brandt.

@Eclips4

Eclips4 commented May 13, 2024•
edited
Loading

Copy link
Copy Markdown
Member

However, I'm wondering why changing README.md triggers our CI/CD actions (I mean actions that build the interpreter and run the test suite). This obviously should not happen for this kind of changes.

This could be easily fixed by changing the .github/workflows/jit.yml:

diff --git a/.github/workflows/jit.yml b/.github/workflows/jit.yml
index 7152cde8f4..1ac2829f0a 100644
--- a/.github/workflows/jit.yml+++ b/.github/workflows/jit.yml@@ -1,6 +1,8 @@
name: JIT
on:
pull_request:
+ paths-ignore:+ - 'Tools/jit/README.md'
paths:
- '**jit**'
- 'Python/bytecodes.c'

Perhaps it's not worth the effort because README is rarely changed.

savannahostrowski
savannahostrowski approved these changes May 14, 2024
View reviewed changes

@savannahostrowskisavannahostrowski 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.

LGTM - thanks for adding this!

ericsnowcurrently
ericsnowcurrently approved these changes May 14, 2024
View reviewed changes

@ericsnowcurrentlyericsnowcurrently 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.

LGTM

@ericsnowcurrently

ericsnowcurrently commented May 14, 2024

Copy link
Copy Markdown
Member

However, I'm wondering why changing README.md triggers our CI/CD actions (I mean actions that build the interpreter and run the test suite). This obviously should not happen for this kind of changes.

This could be easily fixed by changing the .github/workflows/jit.yml:

That makes sense to me. @brandtbucher, what do you think?

erlend-aasland
erlend-aasland reviewed May 14, 2024
View reviewed changes
Comment threadTools/jit/README.md Outdated
@hugovk

hugovk commented May 14, 2024

Copy link
Copy Markdown
Member

This could be easily fixed by changing the .github/workflows/jit.yml:

Could also ignore Tools/jit/mypy.ini

@hroncok@erlend-aasland
Mark the code-block as shell
0af61e0
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
brandtbucher
brandtbucher approved these changes May 16, 2024
View reviewed changes
Comment threadTools/jit/README.md
Install LLVM 18 on Fedora Linux 40 or newer:

```sh
sudo dnf install 'clang(major) = 18' 'llvm(major) = 18'

@brandtbucherbrandtbucherMay 16, 2024

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.

Thanks for this! Just double-checking, is the separate Clang install necessary (it's not bundled with the LLVM install)?

@hroncokhroncokMay 16, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Correct. Installing just LLVM does not install clang. Neither the other way around.

@brandtbucher

brandtbucher commented May 16, 2024

Copy link
Copy Markdown
Member

Also, if somebody wants to open a PR to ignore JIT CI on the following files, go ahead:

  • Tools/jit/README.md
  • Tools/jit/mypy.ini
  • Python/perf_jit_trampoline.c

@savannahostrowski

savannahostrowski commented May 16, 2024

Copy link
Copy Markdown
Member

Happy to do that! @brandtbucher

@miss-islington-app

miss-islington-appBot commented May 16, 2024

Copy link
Copy Markdown

Thanks @hroncok for the PR, and @brandtbucher for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@bedevere-app

bedevere-appBot commented May 16, 2024

Copy link
Copy Markdown

GH-119100 is a backport of this pull request to the 3.13 branch.

@savannahostrowskisavannahostrowski mentioned this pull request May 18, 2024
Closed
@Eclips4Eclips4 mentioned this pull request Jun 12, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aaslanderlend-aaslanderlend-aasland left review comments
@ericsnowcurrentlyericsnowcurrentlyericsnowcurrently approved these changes
@savannahostrowskisavannahostrowskisavannahostrowski approved these changes
@brandtbucherbrandtbucherbrandtbucher approved these changes

Assignees

No one assigned

Labels

skip issueskip newstopic-JIT

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@hroncok@Eclips4@ericsnowcurrently@hugovk@brandtbucher@savannahostrowski@erlend-aasland