Skip to content

polkadot: fix commit hash fetch for version string #159279

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.

Sign up for GitHub

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
kevincox merged 1 commit into from
Feb 11, 2022
Merged

polkadot: fix commit hash fetch for version string #159279

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions pkgs/applications/blockchains/polkadot/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
, llvmPackages
, protobuf
, rustPlatform
, writeShellScriptBin
}:
rustPlatform.buildRustPackage rec {
pname = "polkadot";
Expand All @@ -13,12 +14,38 @@ rustPlatform.buildRustPackage rec {
owner = "paritytech";
repo = "polkadot";
rev = "v${version}";
sha256 = "sha256-TgpQ+nRKkbyQcCwecnhgF70FI+rlDuTy7XHUNhvYy64=";
sha256 = "sha256-NXuYUmo80rrBZCcuISKon48SKyyJrkzCEhggxaJNfBM=";

# see the comment below on fakeGit for how this is used
leaveDotGit = true;
postFetch = ''
( cd $out; git rev-parse --short HEAD > .git_commit )
rm -rf $out/.git
'';
};

cargoSha256 = "sha256-PIORMTzQbMdlrKwuF4MiGrLlg2nQpgLRsaHHeiCbqrg=";

nativeBuildInputs = [ clang ];
nativeBuildInputs =
let
# the build process of polkadot requires a .git folder in order to determine
# the git commit hash that is being built and add it to the version string.
# since having a .git folder introduces reproducibility issues to the nix
# build, we check the git commit hash after fetching the source and save it
# into a .git_commit file, and then delete the .git folder. then we create a
# fake git command that will just return the contents of this file, which will
# be used when the polkadot build calls `git rev-parse` to fetch the commit
# hash.
fakeGit = writeShellScriptBin "git" ''
if [[ $@ = "rev-parse --short HEAD" ]]; then
Copy link
Contributor

@kevincox kevincox Feb 11, 2022

Choose a reason for hiding this comment

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

Do you know where this is set? I think it would be much cleaner and more maintainable to submit an upstream patch to read the version from an environment variable (if this isn't already available). This will be useful for multiple consumers.

Copy link
Contributor

@kevincox kevincox Feb 11, 2022

Choose a reason for hiding this comment

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

Likely here: https://github.com/paritytech/substrate/blob/22ed351d86b3b04ca1df82fb0036c6376bf1fea0/utils/build-script-utils/src/version.rs#L23. It should be simple to make this code just return early if SUBSTRATE_CLI_IMPL_VERSION is already set.

Copy link
Member Author

@andresilva andresilva Feb 11, 2022

Choose a reason for hiding this comment

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

Yes, my plan is to add support in upstream for reading the commit hash from an environment variable. I am one of the maintainers of the project so I don't think there will be any issues getting it through although it will still take some versions to get it released.

Copy link
Member Author

@andresilva andresilva Feb 11, 2022

Choose a reason for hiding this comment

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

It's set here btw https://github.com/paritytech/substrate/blob/master/utils/build-script-utils/src/version.rs#L23, not on polkadot itself but substrate which is a dependency.

Copy link
Contributor

@kevincox kevincox Feb 11, 2022

Choose a reason for hiding this comment

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

Sounds good. Would it be reasonable to file an issue in the upstream project (or in nixpkgs if that is not wanted) and link to it from this comment just to tie everything together and track the removal of the workaround?

cat /build/source/.git_commit
else
>&2 echo "Unknown command: $@"
exit 1
fi
'';
in
[ clang fakeGit ];

LIBCLANG_PATH = "${llvmPackages.libclang.lib}/lib";
PROTOC = "${protobuf}/bin/protoc";
Expand Down