Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Updating the CI system #8765

Merged
merged 16 commits into from
Aug 25, 2018
Merged

Updating the CI system #8765

merged 16 commits into from
Aug 25, 2018

Conversation

General-Beck
Copy link
Contributor

@General-Beck General-Beck commented Jun 2, 2018

Updating the CI system with the publication of releases and binary files on github. example
Removed i686 and armv6 builds.
Removed md5 hash.
Added parity/parity-evm Docker image.
After each binary file is builded for a given architecture, they are automatically packaged for different installers.
Artifacts are created at each stage and stored for a month on the server.
binaries from build stage
packages
List of generated systems and packages:

OS Arch Package
Ubuntu arm64 deb
* arm64 snap
* armv7 deb
* armv7 snap
* amd64 deb
* amd64 snap
Debian amd64 deb
CentOS x86_64 rpm
Windows x86_64 exe (installer)
OSX x86_64 pkg (installer)
Docker x86_64 image

Improved build time with the caching cargo.
Need to test on nightly and tags.
We will need to change the vanity to add files to the download section.

and many more changes...
P.S. now everything has become beautiful!

…les on github

Signed-off-by: Denis S. Soldatov aka General-Beck <general.beck@gmail.com>
@General-Beck General-Beck added A0-pleasereview 🤓 Pull request needs code review. M0-build 🏗 Building and build system. M1-ci 🙉 Continuous integration. M2-installer 📲 Installers for MacOS and Windows. labels Jun 2, 2018
@General-Beck General-Beck self-assigned this Jun 2, 2018
@General-Beck
Copy link
Contributor Author

trigger test

@5chdn 5chdn added this to the 1.12 milestone Jun 2, 2018
@@ -1,7 +1,5 @@
#!/bin/bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we even using this build.sh monstrosity?
I think at least partially the idea was to replace it with smaller and more straightforward task-specific scripts (which it seems like you did). Also, build.sh is never mentioned in the gitlab-ci.yml

fi
aws s3 rm --recursive s3://$S3_BUCKET/$CI_BUILD_REF_NAME/$BUILD_PLATFORM
aws s3api put-object --bucket $S3_BUCKET --key $CI_BUILD_REF_NAME/$BUILD_PLATFORM/parity$S3WIN --body target/$PLATFORM/release/parity$S3WIN
aws s3api put-object --bucket $S3_BUCKET --key $CI_BUILD_REF_NAME/$BUILD_PLATFORM/parity$S3WIN.md5 --body parity$S3WIN.md5
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we even build md5 in our new pipeline?
Let's just stop doing that, and stop caring about it?
sha256 is better in every possible way, let's just stick to it?

Copy link
Collaborator

@kirushik kirushik left a comment

Choose a reason for hiding this comment

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

Overall — looks quite good; I've leaf a couple of comments on some smaller issues above.

@5chdn
Copy link
Contributor

5chdn commented Jun 4, 2018

Okay, thanks @General-Beck. There is only one reasonable way to test this:

* [new tag]             v1.12.0-ci1 -> v1.12.0-ci1

https://gitlab.parity.io/parity/parity/pipelines/20519

@5chdn 5chdn added the A4-awaitingci 🤖 Pull request is waiting for changes on the CI to complete tests before review/merge can begin. label Jun 4, 2018
@5chdn
Copy link
Contributor

5chdn commented Jun 4, 2018

@kirushik
Copy link
Collaborator

kirushik commented Jun 4, 2018

@General-Beck Looks like wasm-tests submodule is not updated properly for tests?

@5chdn
Copy link
Contributor

5chdn commented Jun 5, 2018

@General-Beck is this up to date with master?

@5chdn 5chdn added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. A3-stale 🍃 Pull request did not receive any updates in a long time. No review needed at this stage. Close it. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 13, 2018
@5chdn
Copy link
Contributor

5chdn commented Jul 2, 2018

Assuming you are working on this somewhere else (Gitlab?), I'll close this and we can reopen this once ready. Please keep me in the loop, happy to provide early feedback.

@5chdn 5chdn closed this Jul 2, 2018
@5chdn 5chdn removed A4-awaitingci 🤖 Pull request is waiting for changes on the CI to complete tests before review/merge can begin. A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Aug 20, 2018
.gitlab-ci.yml Outdated
- scripts/gitlab-build.sh i686-unknown-linux-gnu i686-unknown-linux-gnu i386 gcc g++ linux
tags:
- rust-i686
- gitlab-next
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be removed before the merge?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

.gitlab-ci.yml Outdated
only: &publishable_branches
- nightly # Our nightly builds from schedule, on `master`
- "v*" # Our version tags
- gitlab-next
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, seems to be a stub for testing, not something we need to merge.

.gitlab-ci.yml Outdated
image: parity/rust:gitlab-ci
- scripts/gitlab/publish-docker.sh parity-evm

publish:github:s3:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This (and the corresponding script push.sh) are named misleadingly. I can easily imagine us forgetting that we're pushing to both locations from the way this section and accompanying script are called.

Also: why don't we have pushing to S3 and pushing to Github as separate steps?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for separate steps if possible

.gitlab-ci.yml Outdated

####stage: docs

json:rpc:docs:
Copy link
Collaborator

Choose a reason for hiding this comment

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

👏


build_docs() {
echo "__________Build docs__________"
npm install
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's prefer yarn everywhere, shouldn't we?
Dependencies caching should become much more straightforward with it.

commit_files() {
echo "__________Commit files__________"
git checkout -b rpcdoc-update-${CI_COMMIT_REF_NAME}
git commit .
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it intended to be git add here?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes.

Copy link
Contributor

@5chdn 5chdn left a comment

Choose a reason for hiding this comment

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

LGMT, only minor grumbles. I'm in favor of merging this early and break stuff rather than keeping this around much longer :)

.gitlab-ci.yml Outdated
- scripts/gitlab-build.sh i686-unknown-linux-gnu i686-unknown-linux-gnu i386 gcc g++ linux
tags:
- rust-i686
- gitlab-next
Copy link
Contributor

Choose a reason for hiding this comment

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

yes

.gitlab-ci.yml Outdated
.publishable_branches: # list of git refs for publishing builds to the "production" locations
only: &publishable_branches
- nightly # Our nightly builds from schedule, on `master`
- "v*" # Our version tags
Copy link
Contributor

Choose a reason for hiding this comment

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

branch vitalik-improvements would match this, right? what about "v2*" or even regex if possible?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great catch!

- tags
- stable
- triggers
- master
Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean tests are not run on pull requests anymore?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Only tests with <<: *optional_test included; testing with stable happens everywhere (see section test:rust:stable:); testing with beta and nightly Rust versions are only limited to master branch. It makes some sense, but not the perfect one — probably we want to be sure that our code breaks on nightly immediately, not during the next release.

.gitlab-ci.yml Outdated
variables:
CARGO_TARGET: armv7-linux-androideabi

build:macos:x86_64:
Copy link
Contributor

Choose a reason for hiding this comment

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

for consistency, can we rename this to build:darwin:macos:x86_64?

.gitlab-ci.yml Outdated
windows:
<<: *collect_artifacts

build-windows-x86_64:
Copy link
Contributor

Choose a reason for hiding this comment

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

also build:windows:msvc:x86_64?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, windows builds can't have : characters in their names, because windows paths can't have said character.

Maybe let's come up with a better (and universal) separator?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok let's go back to - then

.gitlab-ci.yml Outdated
image: parity/rust:gitlab-ci
- scripts/gitlab/publish-docker.sh parity-evm

publish:github:s3:
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for separate steps if possible


If you continue, Parity will be installed as a user service. You will be able to use the Parity Wallet through your browser by using the menu bar icon, following the shortcut in the Launchpad or navigating to http://localhost:8180/ in your browser.

Parity is distributed under the terms of the GPL."
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this?

set -u # treat unset variables as error

case ${CI_COMMIT_REF_NAME} in
master|*v2.1*|gitlab-next) export CHANNEL="edge";;
Copy link
Contributor

Choose a reason for hiding this comment

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

replace gitlab-next with nightly?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it's sufficient on nightly only, not master

commit_files() {
echo "__________Commit files__________"
git checkout -b rpcdoc-update-${CI_COMMIT_REF_NAME}
git commit .
Copy link
Contributor

Choose a reason for hiding this comment

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

yes.


upload_files() {
echo "__________Upload files__________"
git push --tags
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this include #9219 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

@5chdn 5chdn modified the milestones: 2.0 Ethereum, 2.1 Aug 21, 2018
@5chdn 5chdn added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 21, 2018
Copy link
Contributor

@gabreal gabreal left a comment

Choose a reason for hiding this comment

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

good job, let's give it a try.

curl https://sh.rustup.rs -sSf | sh -s -- -y && \
# rustup directory
PATH=/root/.cargo/bin:$PATH && \
RUN apt update && apt install -y --no-install-recommends openssl libudev-dev file
Copy link
Contributor

Choose a reason for hiding this comment

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

Why libudev-dev? Shouldn't it be libudev?
And there is no dependency on libssl anymore, so why openssl?

cp -v ethkey $SNAPCRAFT_PART_INSTALL/usr/bin/ethkey
cp -v ethstore $SNAPCRAFT_PART_INSTALL/usr/bin/ethstore
cp -v whisper $SNAPCRAFT_PART_INSTALL/usr/bin/whisper
stage-packages: [libc6, libssl1.0.0, libudev1, libstdc++6, cmake]
Copy link
Contributor

Choose a reason for hiding this comment

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

https://docs.snapcraft.io/build-snaps/syntax#stage-packages
so the files from these packages end up in the snap. if this is required it should be the same for the docker image.

- parity.zip
name: "x86_64-apple-darwin_parity"
windows:
<<: *collect_artifacts
Copy link
Contributor

Choose a reason for hiding this comment

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

the previous jobs (e.g. linux:linux:android:armhf) get their *collect_artifacts block from the *build anchor, doesn't that also work for this one?

@5chdn
Copy link
Contributor

5chdn commented Aug 24, 2018

Addressed most of the issues. Will merge once the CI is green. And then we can work from there.

@5chdn
Copy link
Contributor

5chdn commented Aug 24, 2018

Copy link
Collaborator

@kirushik kirushik left a comment

Choose a reason for hiding this comment

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

@5chdn Will this masternightly change do what you actually need? If yes, please merge at will.

.gitlab-ci.yml Outdated
<<: *publish_docker
script:
- scripts/gitlab/publish-docker.sh parity-evm

publish:github:s3:
publish-github-and-s3:
Copy link
Collaborator

Choose a reason for hiding this comment

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

In my ideal world we would split this into two separate steps.
But let's merge it as it is for now.

@@ -3,7 +3,7 @@
set -e # fail on any error
set -u # treat unset variables as error
case ${CI_COMMIT_REF_NAME} in
master|*v2.1*|gitlab-next) export GRADE="devel";;
nightly|*v2.1*) export GRADE="devel";;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we going to publish nightlies only from the special nightly branch? Why not from the master?

Copy link
Contributor

Choose a reason for hiding this comment

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

master is triggered always after we merge a pull request, so multiple times per day and nightly is only happing once per night which is sufficient. now, master does not create a snap which is fine IMHO.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@5chdn Alternatively, we can avoid triggering the actual build-and-package on master on every push (only tests), and trigger the builds only once per day with a cron-like trigger (Gitlab CI already supports those). This way, there would be no need in maintaining a separate nightly branch manually.

If you agree with me that not having to maintain separate nightly is desired, please open a separate issue for this (or let me know, and I'll do this).

Copy link
Contributor

Choose a reason for hiding this comment

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

we are doing that. nightly is a tag.

@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Aug 25, 2018
@5chdn 5chdn merged commit bd3bc5c into master Aug 25, 2018
@5chdn 5chdn deleted the gitlab-next branch August 25, 2018 22:44
@5chdn
Copy link
Contributor

5chdn commented Aug 25, 2018

🎉

dvdplm added a commit that referenced this pull request Aug 30, 2018
* master:
  evmbin: Fix gas_used issue in state root mismatch and handle output better (#9418)
  Update hardcoded sync (#9421)
  Add block reward contract config to ethash and allow off-chain contracts (#9312)
  Private packets verification and queue refactoring (#8715)
  Update tobalaba.json (#9419)
  docs: add parity ethereum logo to readme (#9415)
  build: update rocksdb crate (#9414)
  Updating the CI system  (#8765)
  Better support for eth_getLogs in light mode (#9186)
  Add update docs script to CI (#9219)
  `gasleft` extern implemented for WASM runtime (kip-6) (#9357)
  block view! removal in progress (#9397)
  Prevent sync restart if import queue full (#9381)
  nonroot CentOS Docker image (#9280)
  ethcore: kovan: delay activation of strict score validation (#9406)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. M0-build 🏗 Building and build system. M1-ci 🙉 Continuous integration. M2-installer 📲 Installers for MacOS and Windows. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants