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

[Depends] Cleanup Rust and add multi-arch linux support #2280

Merged
merged 3 commits into from
Aug 21, 2021

Conversation

Fuzzbawls
Copy link
Collaborator

The first commit here comes from zcash/zcash#4725 and provides a significant cleanup in terms of number of files needed in the depends tree.

The second commit re-works the way we fetch the Rust dependencies to allow depends to be built for linux on non-x86_64 systems. Currently I've compiled native toolchains for x86_64, arm, armv7, and aarch64

The third commit addresses #2127 by re-compiling the rust toolchains with static linking.

The fourth commit is temporary, and will be removed prior to merging.

@Fuzzbawls Fuzzbawls added this to the 6.0.0 milestone Apr 1, 2021
@Fuzzbawls Fuzzbawls self-assigned this Apr 1, 2021
@random-zebra
Copy link

I was able to build locally on x86_64, as well as with depends (using the last commit).
Without the last commit depends fails due to sha256sum mismatches for native_rust-1.42.0/rust-1.42.0-x86_64-unknown-linux-gnu.tar.gz.

Also couldn't complete gitian build:

make: Entering directory '/home/ubuntu/build/pivx/depends'
Extracting native_rust...
/home/ubuntu/cache/common/rust-std-1.42.0-i686-unknown-linux-gnu.tar.gz: FAILED
sha256sum: WARNING: 2 computed checksums did NOT match
/home/ubuntu/cache/common/rust-1.42.0-x86_64-unknown-linux-gnu.tar.gz: FAILED
funcs.mk:256: recipe for target '/home/ubuntu/build/pivx/depends/work/build/i686-pc-linux-gnu/native_rust/1.42.0-497ecf3ed6d/.stamp_extracted' failed
make: Leaving directory '/home/ubuntu/build/pivx/depends'
make: *** [/home/ubuntu/build/pivx/depends/work/build/i686-pc-linux-gnu/native_rust/1.42.0-497ecf3ed6d/.stamp_extracted] Error 1

@Fuzzbawls
Copy link
Collaborator Author

Also couldn't complete gitian build:

make: Entering directory '/home/ubuntu/build/pivx/depends'
Extracting native_rust...
/home/ubuntu/cache/common/rust-std-1.42.0-i686-unknown-linux-gnu.tar.gz: FAILED
sha256sum: WARNING: 2 computed checksums did NOT match
/home/ubuntu/cache/common/rust-1.42.0-x86_64-unknown-linux-gnu.tar.gz: FAILED
funcs.mk:256: recipe for target '/home/ubuntu/build/pivx/depends/work/build/i686-pc-linux-gnu/native_rust/1.42.0-497ecf3ed6d/.stamp_extracted' failed
make: Leaving directory '/home/ubuntu/build/pivx/depends'
make: *** [/home/ubuntu/build/pivx/depends/work/build/i686-pc-linux-gnu/native_rust/1.42.0-497ecf3ed6d/.stamp_extracted] Error 1

remove all of the rust* files from gitian-builder/cache/common.

@random-zebra
Copy link

remove all of the rust* files from gitian-builder/cache/common.

Mhm... same thing. I have also tried removing the whole cache folder.
I've noticed that it downloads from https://depends.pivx.org, so it must be the same checksum problem I had when building depends without the last commit.

@Fuzzbawls
Copy link
Collaborator Author

@random-zebra these issues are likely due to a limitation of the gitian-build.py script when trying to build a PR with the -p option that changes depends dependency versions/URLs.

One way around this is to have multiple local repos:

  1. base repo that has the head of the PR checked out
  2. the pivx repo within the gitian-builder/inputs repo that is checked out to the specific HEAD commit hash of the PR

then the build command becomes something like ./gitian-build.py --docker -b -c -u <fs-path-to-base-repo> -D <signer> <local-repo-head-commit-hash>

The -u option forces the script to override the "URL" parameter with a local repository path instead of reading descriptors from the default master branch of the public repo on GitHub, and the -c option, combined with the <local-repo-head-commit-hash> forces the script to explicitly run off the exact head commit rather than a merge commit.

Another option that could be done with this, is spend the time to compile a more recent version of the Rust toolchain for each host OS (avg ~30 hrs each for armhf aarch64, and ~18-22 hrs for i686/x86_64 + time to compile std libraries for the various cross-compile targets). Doing this would, at the very least, avoid package version conflicts (the purpose of the final commit currently) as the version number would change. The downside is that it may introduce unwanted/unexpected results that may require more time to sort out.

@random-zebra
Copy link

Thanks for the explanation. Unfortunately I wasn't able to make it work.
Although, one of the issues here (and the reason why can't build depends locally without the last commit) is that https://depends.pivx.org and https://fuzzbawls.pw/depends-cache are hosting different files for rust-1.42.0-x86_64-unknown-linux-gnu:

5fed7d705e215fc129c4ace8060b5dc1a47e88228ce0249d48f30d769fcb6fe3 (137.4 MB)

vs

fce33280b344ab0fecc55be24632a3c0086fd763f4df1d59013dfc0e49f0356c (144.3 MB)

@Fuzzbawls
Copy link
Collaborator Author

hmm wait...were you trying to do a gitian build WITHOUT the last commit here? if so then it is expected that it won't work. The last commit's sole purpose is to allow testers to do gitian (and local depends) builds before the files are overwritten on depends.pivx.org.

Because this includes a re-build of the 1.42.0 rust toolchain distribution files (re-built for static linking to not have a host dependency on OpenSSL 1.1), the file names are the same as before, but obviously have a different shasums. So I purposely included the (temporary) last commit here that has the new toolchain distribution files hosted on fuzzbawls.pw/depends-cache, as overwriting the existing files on depends.pivx.org prior to review/testing of this PR would mean breaking CI builds for all PRs not based on top of this one.

The intention in doing it this way is to not have unrelated PRs depend on this PR's changes prior to this PR being reviewed/tested. Once this gets approval, I'll need to update the toolchain distribution files on depends.pivx.org, remove the last commit, then merge.

@random-zebra
Copy link

Once this gets approval, I'll need to update the toolchain distribution files on depends.pivx.org, remove the last commit, then merge.

Ok, great, now it's clear. ACK

@Fuzzbawls
Copy link
Collaborator Author

yeah sorry for the confusion...this kind of update is delicate and not really the ideal way of doing things. hopefully in the future we will be able to use the rust official toolchain distribution files once we require a minimum linux ABI for the C library of 2.18 (currently only CentOS 7 is holding this back, and it doesn't reach EOL until 2024...which is also not ideal) and rust doesn't include more ABI breaking symbols.

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Spent some time here, all good 👌. Have verified dependencies hash equality between depends and the cargo.lock file manually and tested it under depends on macOS Mojave.

ACK, ready to go.

@Fuzzbawls
Copy link
Collaborator Author

quick update here: I'm holding off rebasing / merging this until the other 5.3.0 PRs are reviewed/merged. The primary reason for this hold is because once this is merged, EVERY open PR that needs to cycle through a CI run will need to be rebased on top of this just to pick up the new Rust package files, as existing files will be invalidated, so it is logistically best to merge this towards the end of a release cycle.

@furszy
Copy link

furszy commented Aug 7, 2021

this can be release on a point release after merging the priority PRs for v5.3 and/or while v5.3 is being tested. This work is not attached to any release timeframe anyway.

The focus needs to be on validating that everything is working properly after merging the priority PRs 2411 and 2492 and bumping testnet.

@furszy
Copy link

furszy commented Aug 20, 2021

can remove the last commit now, so this one can get merged right after the three small remaining PRs for v5.3.

@Fuzzbawls Fuzzbawls merged commit 7fa6374 into PIVX-Project:master Aug 21, 2021
Fuzzbawls added a commit to Fuzzbawls/PIVX that referenced this pull request Oct 3, 2021
This script became un-necessary with PIVX-Project#2280, but the script file and it's
 reference in the Makefile weren't removed.
Fuzzbawls added a commit to Fuzzbawls/PIVX that referenced this pull request Oct 7, 2021
This script became un-necessary with PIVX-Project#2280, but the script file and it's
 reference in the Makefile weren't removed.
furszy added a commit that referenced this pull request Oct 10, 2021
ec2d592 [Depends] Remove unused cargo-checksum.sh script (Fuzzbawls)

Pull request description:

  This script became un-necessary with #2280, but the script file and it's
   reference in the Makefile weren't removed.

  Extracted from #2583 as this is a trivial change that is otherwise holding up a follow-up
  linting update and doesn't necessarily need to be included in #2583

ACKs for top commit:
  furszy:
    ACK ec2d592
  random-zebra:
    utACK ec2d592

Tree-SHA512: f2754d4c05fd3274f3b0431a7804f40fc3c45ad21eeedc9e6521068ee5125ba92688131b9723b9cf9a833341d2be0277f8acb0fb9ede77c372c357df4c2780aa
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.

3 participants