solc-select, crytic-compile, slither-analyzer, echidna: improve testing on ARM #127681

elopez · 2023-04-06T00:43:38Z

Have you followed the guidelines for contributing?
Have you ensured that your commits follow the commit style guide?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

Running solc (through solc-select) requires an Intel system or Rosetta. The tests therefore fail on non-Rosetta ARM Macs, which causes solc-select and dependents to not be bottled on ARM. To remediate this, account for this limitation in the solc-select test, and implement testing with a Truffle or prebuilt project on ARM instead on crytic-compile, echidna, and slither-analyzer, which can use the solcjs compiler if needed.

SMillerDev · 2023-04-06T07:07:42Z

Formula/solc-select.rb

+    # running solc itself requires an Intel system or Rosetta
+    return if Hardware::CPU.arm?
+


That seems like it won't be very useful for people to have this bottled on ARM then. I'd say it's better to mark it as x86.

Hi @SMillerDev! The software itself (solc-select) is written in Python and can run fine on ARM. It's also a runtime dependency (imported python lib) for some of the other programs in this PR, which can be meaningfully used on an ARM Mac without Rosetta. My understanding is that marking solc-select as x86 would mean none of the packages here get bottled, which would not be desirable.

@SMillerDev Seems fine on me as-is given what @elopez has said.

elopez · 2023-04-07T15:29:10Z

It looks like the CI timed out, we may need a longer run for this PR 😅

MikeMcQuaid · 2023-04-17T10:55:02Z

Formula/crytic-compile.rb

-    system "solc-select", "install", "0.8.0"
-    with_env(SOLC_VERSION: "0.8.0") do
-      system bin/"crytic-compile", testpath/"test.sol", "--export-format=solc", "--export-dir=#{testpath}/export"
+    resource "testdata" do


@Homebrew/core is this a thing we're doing now? Not opposed, kinda in favour if it saves a non-test-time download, but just wanted to check it 1) works and 2) is desirable.

By "this", do you mean resource blocks inside test blocks? IIRC there were a handful of examples of this a while back that we had to get rid of to enable formulae being frozen after instantiation.

Agreed that it would be good to do if it worked though.

Seems to work, though, so I must be misremembering. Or maybe formulae freezing has changed since then. Probably the former. @Bo98?

By "this", do you mean resource blocks inside test blocks?

@carlocab Yup!

Just to add context, I opted to include this resource block inside the test block to avoid having virtualenv_install_with_resources trying to install the test file as a python package.

Formula/crytic-compile.rb

Formula/echidna.rb

Formula/slither-analyzer.rb

Formula/solc-select.rb

MikeMcQuaid · 2023-04-17T10:56:57Z

Formula/solc-select.rb

+    # running solc itself requires an Intel system or Rosetta
+    return if Hardware::CPU.arm?
+


@SMillerDev Seems fine on me as-is given what @elopez has said.

Running solc itself requires an Intel system or Rosetta. Account for that fact in the brew test.

Running solc (through solc-select) requires an Intel system or Rosetta. Implement testing with a precompiled project instead, which should work on all platforms.

Running solc (through solc-select) requires an Intel system or Rosetta. Implement testing with a pre-compiled project instead, which should work across all platforms.

`solc` binaries (through `solc-select`) are only available for Intel platforms or through Rosetta. Switch `echidna` to test with a Truffle project instead, which can be built on all platforms.

elopez · 2023-04-17T13:43:00Z

@MikeMcQuaid thanks for the review! Can you please tag this PR with the longer timeout CI tag? it seems it's very close to the default time limit and some of the jobs got cancelled.

github-actions · 2023-04-17T23:04:46Z

🤖 A scheduled task has requested bottles to be published to this PR.

github-actions · 2023-04-17T23:04:54Z

⚠️ Bottle publish failed. CC @carlocab

bottle publish failed

carlocab

Thanks!

github-actions bot added the python Python use is a significant feature of the PR or issue label Apr 6, 2023

elopez changed the title ~~solc-select, crytic-compile, slither-analyzer: improve testing on ARM~~ solc-select, crytic-compile, slither-analyzer, echidna: improve testing on ARM Apr 6, 2023

elopez marked this pull request as ready for review April 6, 2023 03:18

elopez mentioned this pull request Apr 6, 2023

echidna 2.1.1 #127626

Closed

SMillerDev reviewed Apr 6, 2023

View reviewed changes

elopez requested a review from SMillerDev April 13, 2023 19:35

elopez force-pushed the improve-slither-bottling branch 2 times, most recently from 93c8917 to 698ff17 Compare April 15, 2023 17:51

elopez marked this pull request as draft April 15, 2023 17:51

elopez force-pushed the improve-slither-bottling branch 2 times, most recently from d2d304a to 6ed9a30 Compare April 15, 2023 20:00

elopez marked this pull request as ready for review April 15, 2023 21:53

SMillerDev requested a review from a team April 17, 2023 07:51

MikeMcQuaid reviewed Apr 17, 2023

View reviewed changes

elopez added 4 commits April 17, 2023 08:36

solc-select: improve testing on ARM

a790938

Running solc itself requires an Intel system or Rosetta. Account for that fact in the brew test.

crytic-compile: improve testing on ARM

e045755

Running solc (through solc-select) requires an Intel system or Rosetta. Implement testing with a precompiled project instead, which should work on all platforms.

slither-analyzer: improve testing on ARM

1e36c79

Running solc (through solc-select) requires an Intel system or Rosetta. Implement testing with a pre-compiled project instead, which should work across all platforms.

echidna: improve testing on ARM

2aa150d

`solc` binaries (through `solc-select`) are only available for Intel platforms or through Rosetta. Switch `echidna` to test with a Truffle project instead, which can be built on all platforms.

elopez force-pushed the improve-slither-bottling branch from 6ed9a30 to 2aa150d Compare April 17, 2023 11:37

MikeMcQuaid previously approved these changes Apr 17, 2023

View reviewed changes

MikeMcQuaid added CI-long-timeout Use longer GitHub Actions CI timeout. long build Needs CI-long-timeout labels Apr 17, 2023

elopez force-pushed the improve-slither-bottling branch from 8ab7a62 to 2aa150d Compare April 17, 2023 16:05

carlocab added the CI-no-bottles Merge without publishing bottles label Apr 18, 2023

carlocab approved these changes Apr 18, 2023

View reviewed changes

carlocab removed the CI-long-timeout Use longer GitHub Actions CI timeout. label Apr 18, 2023

carlocab added this pull request to the merge queue Apr 18, 2023

Merged via the queue into Homebrew:master with commit 93904aa Apr 18, 2023
29 of 37 checks passed

elopez mentioned this pull request Apr 18, 2023

echidna 2.1.1 #128647

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

solc-select, crytic-compile, slither-analyzer, echidna: improve testing on ARM #127681

solc-select, crytic-compile, slither-analyzer, echidna: improve testing on ARM #127681

elopez commented Apr 6, 2023 •

edited

SMillerDev Apr 6, 2023

elopez Apr 6, 2023 •

edited

MikeMcQuaid Apr 17, 2023

elopez commented Apr 7, 2023

MikeMcQuaid Apr 17, 2023

carlocab Apr 17, 2023 •

edited

carlocab Apr 17, 2023

MikeMcQuaid Apr 17, 2023

elopez Apr 17, 2023

MikeMcQuaid Apr 17, 2023

elopez commented Apr 17, 2023

github-actions bot commented Apr 17, 2023

github-actions bot commented Apr 17, 2023

carlocab left a comment

		# running solc itself requires an Intel system or Rosetta
		return if Hardware::CPU.arm?

solc-select, crytic-compile, slither-analyzer, echidna: improve testing on ARM #127681

solc-select, crytic-compile, slither-analyzer, echidna: improve testing on ARM #127681

Conversation

elopez commented Apr 6, 2023 • edited

SMillerDev Apr 6, 2023

Choose a reason for hiding this comment

elopez Apr 6, 2023 • edited

Choose a reason for hiding this comment

MikeMcQuaid Apr 17, 2023

Choose a reason for hiding this comment

elopez commented Apr 7, 2023

MikeMcQuaid Apr 17, 2023

Choose a reason for hiding this comment

carlocab Apr 17, 2023 • edited

Choose a reason for hiding this comment

carlocab Apr 17, 2023

Choose a reason for hiding this comment

MikeMcQuaid Apr 17, 2023

Choose a reason for hiding this comment

elopez Apr 17, 2023

Choose a reason for hiding this comment

MikeMcQuaid Apr 17, 2023

Choose a reason for hiding this comment

elopez commented Apr 17, 2023

github-actions bot commented Apr 17, 2023

github-actions bot commented Apr 17, 2023

carlocab left a comment

Choose a reason for hiding this comment

elopez commented Apr 6, 2023 •

edited

elopez Apr 6, 2023 •

edited

carlocab Apr 17, 2023 •

edited