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.

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

Jump to bottom

Bytecode comparison on CircleCI #10039

Merged
merged 5 commits into from
Oct 15, 2020
Merged

Bytecode comparison on CircleCI #10039

merged 5 commits into from
Oct 15, 2020

Conversation

ekpyron
Copy link
Member

@ekpyron ekpyron commented Oct 14, 2020

No description provided.

@ekpyron ekpyron force-pushed the bytecodeComparison branch 15 times, most recently from 181c82a to f761ee0 Compare October 14, 2020 20:02
@ekpyron
Copy link
Member Author

ekpyron commented Oct 14, 2020

Well, that's annoying...
Apparently CircleCI doesn't let us attach workspaces containing conflicting files to the same job.
But it also doesn't allow us to specify which files to attach and workspaces are "append only".

So far the only way to get around this would be to use separately named build directories for all jobs and persist with them in the path. Which is, well, annoying...

@ekpyron
Copy link
Member Author

ekpyron commented Oct 14, 2020

For reference: https://circleci.com/blog/deep-diving-into-circleci-workspaces/
From what I can see they really provide no means around this whatsoever.
So what I'm doing now is just moving the osx workspace files into the subdirectory build/, while for the others remain as they were. That's hacky, but the quickest solution I could come up with.

@ekpyron
Copy link
Member Author

ekpyron commented Oct 14, 2020

Ok, we do have an discrepancy, but probably not a serious one. The version numbers are:

ems: 0.7.4-develop.2020.10.14+commit.d2eef326
win: 0.7.4-develop.2020.10.14+commit.d2eef326
osx: 0.7.4-nightly.2020.10.14+commit.d2eef326
ubu: 0.7.4-nightly.2020.10.14+commit.d2eef326

So half of them call themselves nightly, the other half develop builds, apparently. I'll figure out where this is set...

@ekpyron ekpyron force-pushed the bytecodeComparison branch from d2eef32 to 58a1ec4 Compare October 14, 2020 22:03
@ekpyron
Copy link
Member Author

ekpyron commented Oct 14, 2020

Ok - since we only actually build emscripten nightlies, I see no reason why the CI script should have its builds call themselves nightlies. Let's see if that does it.

@ekpyron ekpyron changed the title [WIP] bytecode comparison Bytecode comparison on CirclecI Oct 14, 2020
@ekpyron ekpyron changed the title Bytecode comparison on CirclecI Bytecode comparison on CircleCI Oct 14, 2020
@ekpyron
Copy link
Member Author

ekpyron commented Oct 14, 2020

I also tried to have the b_win build produce a release binary, when built on tags - otherwise the bytecode comparison would start breaking on releases :-). But we'll probably just need to see if that's enough at the next release.

@ekpyron
Copy link
Member Author

ekpyron commented Oct 14, 2020

So... what about merging this like it is, s.t. we do have bytecode comparisons again, and then let @aarlt and @cameel figure out how to do this more nicely in the next two weeks ;-)? Un-drafting this.

@ekpyron ekpyron marked this pull request as ready for review October 14, 2020 22:43
@ekpyron ekpyron force-pushed the bytecodeComparison branch from 16668fe to 4215986 Compare October 14, 2020 22:44
@ekpyron ekpyron force-pushed the bytecodeComparison branch from 4215986 to a7b8ad3 Compare October 14, 2020 22:49
@ekpyron ekpyron requested a review from cameel October 14, 2020 22:54
ekpyron
ekpyron commented Oct 15, 2020
View reviewed changes
.circleci/config.yml
Comment on lines +597 to +600
paths:
- build/solc/solc
- build/test/soltest
- build/test/tools/solfuzzer
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the main workaround. osx moves these files in a subdirectory build, while the rest has one directory level less.

chriseth
chriseth reviewed Oct 15, 2020
View reviewed changes
scripts/ci/build.sh
@@ -5,7 +5,7 @@ ROOTDIR="$(dirname "$0")/../.."
cd "${ROOTDIR}"

# shellcheck disable=SC2166
if [ "$CIRCLE_BRANCH" = release -o -n "$CIRCLE_TAG" -o -n "$FORCE_RELEASE" ]; then echo -n >prerelease.txt; else date -u +"nightly.%Y.%-m.%-d" >prerelease.txt; fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do we make sure that this is called "nightly" for the actual emscripten build?

Copy link
Member Author

@ekpyron ekpyron Oct 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Those are built in solc-bin from this github action https://github.com/ethereum/solc-bin/blob/gh-pages/.github/workflows/nightly-emscripten.yml

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm... which just calls back scripts/travis-emscripten/build_emscripten.sh ultimately...

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we actually don't :-) - I'll have a look and make sure - in any case, they won't involve this script, so they're unaffected by the change here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we don't - they are called develop... but that needs to be fixed in solc-bin. I'll create a PR there.

@chriseth chriseth merged commit 8b8cd9f into develop Oct 15, 2020
@chriseth chriseth deleted the bytecodeComparison branch October 15, 2020 12:59
This was referenced Oct 30, 2020
Closed
Closed
Closed
Closed
@cameel cameel mentioned this pull request Nov 20, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chriseth chriseth chriseth approved these changes

@cameel cameel Awaiting requested review from cameel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ekpyron @chriseth