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
Doc: Update Gitian (Reproducible Builds) with scripts and small fixes #7658
Doc: Update Gitian (Reproducible Builds) with scripts and small fixes #7658
Conversation
contrib/gitian/README.md
Outdated
@@ -83,7 +87,7 @@ Docker | |||
Prepare for building with docker: | |||
|
|||
```bash | |||
sudo apt-get install git make curl docker.io | |||
sudo apt-get update && sudo apt-get upgrade -y && sudo apt-get install make autoconf g++ libtool pkg-config gperf cmake python3-zmq libdbus-1-dev libharfbuzz-dev git curl docker.io |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive use of sudo. Should combine all the commands into one sudo invocation, or just use su, or sudo bash.
None of these extra packages are needed on the host system. They're only needed inside the docker container, and the docker build scripts already pull them down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, these are the dependencies for if you want to build with depends only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive use of sudo. Should combine all the commands into one sudo invocation, or just use su, or sudo bash.
Will try.
None of these extra packages are needed on the host system. They're only needed inside the docker container, and the docker build scripts already pull them down.
I tried twice with the previous set of packages and got the same error - missing tooling during Android compilation. But I agree, that I should minimize the list better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this would mean, that the Dockerfile has failed to download the Android's dependencies. I will look there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue hasn't repeated anymore after I simply made apt update and upgrade.
contrib/gitian/README.md
Outdated
@@ -233,14 +229,15 @@ This will create a `.sig` file for each `.assert` file above (2 files for each p | |||
Submitting your signed assert files | |||
----------------------------------- | |||
|
|||
Make a pull request (both the `.assert` and `.assert.sig` files) to the | |||
[monero-project/gitian.sigs](https://github.com/monero-project/gitian.sigs/) repository: | |||
Fork the [monero-project/gitian.sigs](https://github.com/monero-project/gitian.sigs/) repository and clone the fork. Add both the `.assert` and `.assert.sig` files, as well as your public GPG key (`.asc`) to `gitian-pubkeys` directory. After pushing, create a PR to merge to the upstream. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're combining the steps for 1st time submission and subsequent submissions here. The original text is basically correct for subsequent submissions, just omitting the explicit steps of cloning the repo and adding your PGP key.
It would be best if you separate out these initial steps from the subsequent steps, since clearly nobody needs to clone and resubmit PGP key on future submissions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following Note: at line 243 already talks about submitting your PGP key, so not sure it even needs to be stated here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And overall, this isn't the place to teach people how to make pull requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will rethink it, but the original didn't work out very well here.
And overall, this isn't the place to teach people how to make pull requests.
Overall I agree, but if a one or two superfluous sentences spare 10 persons the trouble and convinces them not to give up, I think it's worth it.
contrib/gitian/README.md
Outdated
git commit -S -a -m "Add $GH_USER $VERSION" | ||
git push --set-upstream $GH_USER $VERSION | ||
git push --set-upstream origin $VERSION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fwiw, I usually clone monero-project/foo as the origin, and my fork'd repo is named "mine". It's handier to have monero-project as the origin, because then you can do a single git pull
later to bring yourself up to date. And then when submitting your files just git push mine $VERSION
which will give you a link to create the pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the initial steps should be: fork gitian.sigs repo on github. Then:
git clone https://github.com/monero-project/gitian.sigs/
cd gitian.sigs
git remote add $GH_USER https://github.com/$GH_USER/gitian.sigs
Followed by the original text.
contrib/gitian/README.md
Outdated
``` | ||
|
||
Where `GH_USER` is your Github user name and `VERSION` is the version tag you want to build. | ||
Where `GH_USER` is your Github user name and `VERSION` is the version tag you want to build. The `TARGET_LIST` will simplify your commands after building. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no more TARGET_LIST
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep.
gpg --detach-sign ${VERSION}-osx/${GH_USER}/monero-osx-*-build.assert | ||
gpg --detach-sign ${VERSION}-android/${GH_USER}/monero-android-*-build.assert | ||
gpg --detach-sign ${VERSION}-freebsd/${GH_USER}/monero-freebsd-*-build.assert | ||
for ASSERT in sigs/${VERSION}-*/*/*.assert; do gpg --detach-sign ${ASSERT}; done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I guess the sigs/
prefix is not needed here, if you're doing this in the gitian.sigs repo and not in the gitianuser out/ directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your sig/*
version is correct. The previous script (verification) will only temporarily jump into the out/
dir.
I'm still trying to isolate the missing package issue. It went all fine under Ubuntu 18.04 Server each time. I'm retrying under Ubuntu 18.04 Desktop where it first occurred. |
It seems that apt update & upgrade did the trick. I will wait until the build finishes, then I'll rebase and "publish" the PR. |
4b86900
to
5238863
Compare
contrib/gitian/README.md
Outdated
git checkout -b $VERSION | ||
# add your assert and sig files... | ||
git commit -S -a -m "Add $GH_USER $VERSION" | ||
git push --set-upstream $GH_USER $VERSION | ||
git push --set-upstream origin $VERSION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the repo setup we discussed here, this line should be left as the original text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
5238863
to
834e23b
Compare
…and small code snippets.