Skip to content
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

Update revision file generation #748

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Update revision file generation #748

merged 2 commits into from
Nov 2, 2023

Conversation

burgerrg
Copy link
Contributor

@burgerrg burgerrg commented Nov 2, 2023

  • In version 9, the revision file has a second line naming the revision control system. This pull request adds "git" in the second line.
  • The build process now properly identifies the .git file when used as a submodule.
  • BUILDING is updated to reflect the unconditional inclusion of boot/pb.

@mflatt
Copy link
Contributor

mflatt commented Nov 2, 2023

This generally looks good to me.

The reason for the old loop to search for .git in ancestor directories (instead of assuming ..) was to work when the ChezScheme directory is vendored into another repo, such as racket/racket. I don't feel strongly about this, though, and the racket/racket vendored ChezScheme could just have a difference here.

@burgerrg
Copy link
Contributor Author

burgerrg commented Nov 2, 2023

OK, I think the loop does the same logic that git describe uses internally, so see what you think now.

@burgerrg
Copy link
Contributor Author

burgerrg commented Nov 2, 2023

@mflatt, is it possible for c/build.zuo to catch the non-0 exit code when git describe fails and then write the literal string? That would avoid the loop entirely.

@mflatt
Copy link
Contributor

mflatt commented Nov 2, 2023

@burgerrg The revised loop looks right to me.

The process-status function gets the exit code for a completed process, so that could be used to detect whether a git command succeeded. I prefer not trying to run git if the source does not appear to be in a Git repository, but I can see either choice.

@burgerrg burgerrg merged commit 92c697d into main Nov 2, 2023
21 of 27 checks passed
@burgerrg burgerrg deleted the revision branch November 2, 2023 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants