Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
brandtbucher merged 2 commits into from
May 16, 2024
Merged

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

brandtbucher merged 2 commits into from
May 16, 2024

Conversation

@hroncok
Copy link
Contributor

@hroncokhroncok commented May 13, 2024

@hroncok
Copy link
ContributorAuthor

hroncok commented May 13, 2024

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

@Eclips4
Copy link
Member

Eclips4 commented May 13, 2024•
edited
Loading

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.

@hroncokhroncok changed the base branch from 3.13 to mainMay 13, 2024 10:32
@Eclips4Eclips4 added the needs backport to 3.13 bugs and security fixes label May 13, 2024
@Eclips4
Copy link
Member

Eclips4 commented May 13, 2024•
edited
Loading

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
Copy link
Member

Eclips4 commented May 13, 2024•
edited
Loading

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
Copy link
Member

@savannahostrowskisavannahostrowski left a comment

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
Copy link
Member

@ericsnowcurrentlyericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented May 14, 2024

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
@hugovk
Copy link
Member

hugovk commented May 14, 2024

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
Tools/jit/README.md
Install LLVM 18 on Fedora Linux 40 or newer:

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

@brandtbucherbrandtbucherMay 16, 2024

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)?

Copy link
ContributorAuthor

@hroncokhroncokMay 16, 2024

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
Copy link
Member

brandtbucher commented May 16, 2024

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
Copy link
Member

savannahostrowski commented May 16, 2024

Happy to do that! @brandtbucher

@brandtbucherbrandtbucher merged commit ab73bcd into python:mainMay 16, 2024
@miss-islington-app
Copy link

miss-islington-appbot commented May 16, 2024

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 16, 2024
@hroncok@erlend-aasland@miss-islington
Explain how to install LLVM on Fedora (pythonGH-118983)
98bedc5
(cherry picked from commit ab73bcd) Co-authored-by: Miro Hrončok <miro@hroncok.cz> Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@bedevere-app
Copy link

bedevere-appbot commented May 16, 2024

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

@bedevere-appbedevere-appbot removed the needs backport to 3.13 bugs and security fixes label May 16, 2024
@hroncokhroncok deleted the patch-2 branch May 16, 2024 18:09
@savannahostrowskisavannahostrowski mentioned this pull request May 18, 2024
Closed
@Eclips4Eclips4 mentioned this pull request Jun 12, 2024
Merged
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@hroncok@erlend-aasland@estyxx
Explain how to install LLVM on Fedora (pythonGH-118983)
474b094
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
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