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

SCons: Fix build fetching git_timestamp if git log.showsignature=true #94115

Merged

Conversation

Zorvalt
Copy link
Contributor

@Zorvalt Zorvalt commented Jul 9, 2024

Hi!

This PR fixes the build system if git config has the log.showsignature option set to true.

Without it, for me, the core/version_hash.gen.cpp file was generate like this:

#include "core/version.h"

const char *const VERSION_HASH = "16f98cd7079c2b22248ec358371f17bca355e42e";
const uint64_t VERSION_TIMESTAMP = gpg: Signature faite le lun 08 jui 2024 19:13:45 CEST
gpg:                avec la clef RSA E7E863960AC92F726F9A45CDC3336907360768E1
gpg:                issuer "[email protected]"
gpg: Impossible de vérifier la signature : Pas de clef publique
1720458825;

@Zorvalt Zorvalt requested a review from a team as a code owner July 9, 2024 05:56
@akien-mga akien-mga changed the title Fix build fetching git_timestamp if git log.showsignature=true SCons: Fix build fetching git_timestamp if git log.showsignature=true Jul 9, 2024
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Makes sense. It's too bad that even though we use a very explicit --pretty format there are still other options which can affect the reproducibility of the command.

From a quick look at the git log manual I don't immediately spot other options which may be set in system/global config and affect that, so this might be good enough.

Otherwise an option would be to set GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null in the environment for this command (docs), but I'm not sure how well that would work on Windows, so we can start with just --no-show-signature.

We could also parse the output and make sure to only extract the timestamp with some regex to be safe.

@akien-mga akien-mga added this to the 4.3 milestone Jul 9, 2024
@akien-mga akien-mga merged commit b4943e1 into godotengine:master Jul 9, 2024
18 checks passed
@akien-mga
Copy link
Member

akien-mga commented Jul 9, 2024

Thanks! And congrats for your first merged Godot contribution 🎉

@Zorvalt Zorvalt deleted the fix-git-timestamp-with-show-signature branch July 9, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants