-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
Bump LLVM to 19.1.7 #1653
Bump LLVM to 19.1.7 #1653
Conversation
Shouldn't
|
Done. Thanks! |
The CI failure seems odd and not directly related to this change? |
It should be an issue with my local Git repository, but I don't know exactly where the problem lies. |
I don't think that that's it - the failure is on the GitHub CI side so should have nothing to do with your repos... I'll try a build later to see if I can spot anything. |
It seems to be affected by the |
I don't think that simply removing the shallow clone/checkout is the correct solution here as it undoes changes recently applied here: Maybe @mickflemm might have some better idea? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hope this helps!
.github/workflows/build.yaml
Outdated
@@ -50,7 +50,7 @@ jobs: | |||
|
|||
- name: Checkout required submodules | |||
if: steps.smcache.outputs.cache-hit != 'true' | |||
run: git submodule update --init -j $(nproc) --depth 1 $(echo ${submodule_paths} | sed '$d' | tr '\n' ' ') | |||
run: git submodule update --init -j $(nproc) $(echo ${submodule_paths} | sed '$d' | tr '\n' ' ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in the PR conversation I don't think that this is the correct solution since the shallow clone/checkout was deliberately added recently for a specific purpose:
Ditto for the removal of shallow = true
in .gitmodules
below.
[submodule "llvm"] | ||
path = llvm | ||
url = https://github.com/llvm/llvm-project.git | ||
branch = release/18.x | ||
shallow = true | ||
branch = release/19.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Branch release/19.x
:
is commit 61c9f97
:
But this PR changes the submodule commit from 3b5b5c
to e21dc4b
.
Isn't this a mismatch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The branch release/19.x
has tag llvmorg-19.1.6
( e21dc4b
, https://github.com/llvm/llvm-project/tree/llvmorg-19.1.6 ) . When I submitted (including now), there was no llvmorg-19.1.7
(61c9f97
) related tag. I will update it as soon as it becomes available.
.gitmodules
Outdated
@@ -2,56 +2,44 @@ | |||
path = binutils | |||
url = https://sourceware.org/git/binutils-gdb.git | |||
branch = binutils-2_43-branch | |||
shallow = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See earlier review comment about removing the shallow clone/checkout functionality.
I tried to bump LLVM locally to see if I could reproduce the CI failure but I hit other issues along the way...
I've tried various other approaches to updating the submodule to the I don't understand why the CI build fails because it cannot find the relevant Binutils commit when the shallow clone/checkout is used. And I can't see how bumping LLVM would trigger this. FWIW I cannot reproduce the problem with the "missing/unavailable" Binutils commit if I just clone, configure and build as usual from the repo master/head. |
Many thanks to @TommyMurphyTM1234 for the patient response. Indeed, the |
I don't undestand this. Is the link correct? It seems to link to a CI run status. What approach are you referring to? |
Sorry, I didn't express myself clearly. At that time, I removed |
I've just created #1655 (as a quick workaround). |
The latest failure: presumably depends on this PR being merged first? |
Please rebase this PR to fix the CI issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks! |
No description provided.