Skip to content

Conversation

timon-schelling
Copy link
Member

  • Prevents panic that happens when graphite was build in an environment without git. (len("unkonwn") < 8)
  • Adds option to set release information environment variables, useful for build environments where git is not available.
  • Tell cargo when the build script needs to be run again.
  • Improve readability of build script.

if !gh.trim().is_empty() {
gh.trim().to_string()
} else {
git_or_unknown(&["rev-parse", "--abbrev-ref", "HEAD"])
Copy link
Member

Choose a reason for hiding this comment

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

Why --abbrev-ref?

Copy link
Member Author

Choose a reason for hiding this comment

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

Returns a commit hash otherwise. But I could also use name-rev like you did before.

Copy link
Member

Choose a reason for hiding this comment

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

What does it return if not a commit hash?

Copy link
Member Author

Choose a reason for hiding this comment

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

Branch name. (Rev short name if you want to use git wording)

Copy link
Member

@Keavon Keavon Sep 5, 2025

Choose a reason for hiding this comment

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

We should aim to always have a commit hash so it's possible to actually look up which commit a build link belongs to. (This is also something we don't do on https://editor.graphite.rs but should do.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Commit hash is collected in the statement above. There rev-parse without --abbrev-ref.

Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of this second one?

Copy link
Member Author

Choose a reason for hiding this comment

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

// Collect commit hash
        let commit_hash = env_or_else("GRAPHITE_GIT_COMMIT_HASH", || git_or_unknown(&["rev-parse", "HEAD"]));
// Collect Branch name
         let commit_branch = env_or_else("GRAPHITE_GIT_COMMIT_BRANCH", || { 
                 let gh = env::var("GITHUB_HEAD_REF").unwrap_or_default();

// Code you comment on is used to collect the branch name when it is not set by GitHub locally for now example

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I wasn't looking closely enough. Disregard my last two comments. Anyway, I think we're good to merge.

@Keavon Keavon changed the title Improve release info build script Allow the release info build script to work without requiring Git be installed Sep 5, 2025
@Keavon Keavon changed the title Allow the release info build script to work without requiring Git be installed Allow the release info build script to work without requiring Git to be installed Sep 5, 2025
@timon-schelling timon-schelling merged commit c081d0a into master Sep 5, 2025
4 checks passed
@timon-schelling timon-schelling deleted the improve-release-info-build-script branch September 5, 2025 09:09
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