fix(ci): add sanity check to build.rs scripts, fixed "detected dubious ownership in repository" errors on git
#4986
Conversation
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
jkomyno
requested review from
SevInf and
aqrln
and removed request for
a team and
SevInf
SevInf
approved these changes
Aug 23, 2024
CodSpeed Performance ReportMerging #4986 will not alter performanceComparing Summary
|
Contributor
WASM Query Engine file Size
|
jkomyno
changed the title
git
aqrln
reviewed
Aug 24, 2024
aqrln
reviewed
Aug 24, 2024
Comment on lines
3
to
17
| fn store_git_commit_hash() { | ||
| let output = Command::new("git").args(["rev-parse", "HEAD"]).output().unwrap(); | ||
|
|
||
| // Sanity check on the output. | ||
| if !output.status.success() { | ||
| panic!( | ||
| "Failed to get git commit hash.\nstderr: \n{}\nstdout {}\n", | ||
| String::from_utf8(output.stderr).unwrap_or_default(), | ||
| String::from_utf8(output.stdout).unwrap_or_default(), | ||
| ); | ||
| } | ||
|
|
||
| let git_hash = String::from_utf8(output.stdout).unwrap(); | ||
| println!("cargo:rustc-env=GIT_HASH={git_hash}"); | ||
| } |
Member
There was a problem hiding this comment.
nit: would be nicer to have this defined once in some kind of build utils crate that these three crates will depend on as a build dependency
Contributor
Author
There was a problem hiding this comment.
Yes, the whole store_git_commit_hash could be abstracted
Contributor
Author
There was a problem hiding this comment.
Nit postponed to https://github.com/prisma/team-orm/issues/1261
aqrln
added a commit
that referenced
this pull request
Aug 27, 2024
Follow-up to #4986. * Change the `build.rs` scripts to pass through the `GIT_HASH` environment variable if it's already set. * Set dummmy `GIT_HASH` value in the Nix packages used in the engines size dashboard. * Consistently use the new logic everywhere. Fixes: #4991 Closes: prisma/team-orm#1261
jkomyno
pushed a commit
that referenced
this pull request
Aug 28, 2024
…4992) Follow-up to #4986. * Change the `build.rs` scripts to pass through the `GIT_HASH` environment variable if it's already set. * Set dummmy `GIT_HASH` value in the Nix packages used in the engines size dashboard. * Consistently use the new logic everywhere. Fixes: #4991 Closes: prisma/team-orm#1261
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR checks that
git rev-parse HEAD, the CLI command used at compilation time to store the hash in the Query Engine and Schema Engine, exits successfully.It also complements #4947 by running
git config --global --add safe.directory /root/buildwhen cross-building viaprismagraphql/build:*. This avoids "fatal: detected dubious ownership in repository at /root/build" errors in Linux Musl ARM64 and Linux Static ARM64 architectures.This was introduced after third-party clients notified us that
./query-engine --versionno longer displayed the version hash in Linux Alpine.Fixes: prisma/prisma#25020
Fixes: prisma/prisma#24971