-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Make update_*.py update readme's LLVM version #27254
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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.
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.
Can't we determine this branch name by looking that the
git log?Maybe
git describe(or something like it) should always showemscripten-libs-xx?Ah yes I think you can do
git branch --contains HEADto show all the branchs that contains HEAD, there should always be one of them calledemscripten-libs-xxThere 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 I said in #27254 (comment), my local branch name while I'm working on this is not
emscripten-libs-22, because it is a work in progress. In my case that isemscripten-libs-22-downstream, but it may be some temporary names likechangesfor others.git describefinds the most recent tag reachable from a commit and creates a human-readable string. In my case it returnsllvmorg-22.1.8-1-gf4d109d42671(notemscripten-libs-22-downstreamor whatever) so it's not related to a branch name.Again, for me this is
emscripten-libs-22-downstream, notemscripten-libs-22. The branch name in my local LLVM directory is notemscripten-libs-22. I can't make itemscripten-libs-22because this is a work in progress. After fixing all tests I'll push all final changes to the LLVM directory and commit them toemscripten-libs-22.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.
But can't we make it so that all local changes are always based on some
emscripten-libs-XX?At least for followup changes they should always be true right?
Maybe for the initial change you could create the initial
emscripten-libs-22branch and point it directly atllvmorg-22.X.Yand then haveemscripten-libs-22-downstream(your local work) be branch that targets that?If its too complicated than no worries. I'm not a huge fan of parsing the llvm CMakeFile here but maybe its the best way?
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.
What do you mean by "based on"? My local branch
emscripten-libs-22-downstreamis based on, or on top of,emscripten-libs-22. But the branch name itself is notemscripten-libs-22andgit branch --contains HEADdoes not returnemscripten-libs-22. If I we really want to make the branch nameemscripten-libs-22while I'm working on this, I shouldn't even locally commit my cherry-picked changes to my LLVM directory.This is my current
git logresult:Not sure what "have
emscripten-libs-22-downstream(yur local work) be branch that targets that" means. See mygit logresult. It's on top ofemscripten-libs-22.I don't know where else we can get the full version number. (Parsing that cmake file was Gemini's suggestion) Do you suggest elsewhere?
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.
Ah I see.
I think I was misunderstand what
git branch --contains HEADdid. I was hoping thatgit branch --contains HEADwould print (in your case)emscripten-libs-22-downstreamandemscripten-libs-22andrelease-22.xbut it looks like--containsdoesn't actually do that.You can use it find all the branchs from
release-22.xI guess:But that isn't what we want here. What I was hoping for was a way to find find all the branches that exist in the history of HEAD.. but maybe there is no easy way to get that.
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.
I don't think we need too much high-tech (?) solution here anyway, and as long as we don't change our LLVM library branch name convention (
emscripten-libs-NN) this will work. If we ever change it we can do an one-time update to the script too.