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

solc: 0.8.13 -> 0.8.19 #219240

Merged
merged 1 commit into from
Apr 4, 2023
Merged

solc: 0.8.13 -> 0.8.19 #219240

merged 1 commit into from
Apr 4, 2023

Conversation

fare
Copy link
Contributor

@fare fare commented Mar 2, 2023

Description of changes

Latest upstream release, including many security fixes.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested review from akru, sifmelcara, lionello and dbrock March 2, 2023 21:31
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Mar 2, 2023
@Philipp-M
Copy link
Member

Result of nixpkgs-review pr 219240 run on x86_64-linux 1

1 package built:
  • solc

Copy link
Member

@Philipp-M Philipp-M left a comment

Choose a reason for hiding this comment

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

LGTM, I would advocate to merge this, as the current solc on master seems to be broken.

@figsoda figsoda added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 10, 2023
@fare
Copy link
Contributor Author

fare commented Mar 19, 2023

Fixes #182498

@fare
Copy link
Contributor Author

fare commented Mar 19, 2023

Fixes #122568

@viraptor
Copy link
Contributor

Testing still fails with ModuleNotFoundError: No module named 'requests' on linux

@fare
Copy link
Contributor Author

fare commented Mar 19, 2023

Testing still fails with ModuleNotFoundError: No module named 'requests' on linux

Weird. It passed for me locally. I rebased to latest master in case it helps.

@fare
Copy link
Contributor Author

fare commented Mar 19, 2023

Aha, I need to tweak the python3 in nativeCheckInputs to add some python packages so solc.passthru.tests will work, when solc without tests does work already. Will run the tests...

@fare
Copy link
Contributor Author

fare commented Mar 19, 2023

How with nix-shell do I retry the tests with additional dependencies without rebuilding everything from scratch every time I change something?

@fare
Copy link
Contributor Author

fare commented Mar 19, 2023

Latest issue: some tests really want to execute in a tty. How do I run them in a pty?

OK
Testing LSP...(B
Traceback (most recent call last):
  File "/build/source/./scripts/../test/lsp.py", line 31, in <module>
    tty.setcbreak(sys.stdin.fileno())
  File "/nix/store/0n4y44dnaxafqs7cg625aldrb152x7bx-python3-3.10.10/lib/python3.10/tty.py", line 32, in setcbreak
    mode = tcgetattr(fd)
termios.error: (25, 'Inappropriate ioctl for device')
error: builder for '/nix/store/l74pnb5kla4ivqdarbwahp6q6r0fcacn-solc-0.8.19.drv' failed with exit code 1;
       last 10 log lines:
       > Ran 77 tests in 0.018s
       >
       > OK   
       > Testing LSP...(B
       > Traceback (most recent call last):
       >   File "/build/source/./scripts/../test/lsp.py", line 31, in <module>
       >     tty.setcbreak(sys.stdin.fileno())
       >   File "/nix/store/0n4y44dnaxafqs7cg625aldrb152x7bx-python3-3.10.10/lib/python3.10/tty.py", line 32, in setcbreak
       >     mode = tcgetattr(fd)
       > termios.error: (25, 'Inappropriate ioctl for device')
       For full logs, run 'nix log /nix/store/l74pnb5kla4ivqdarbwahp6q6r0fcacn-solc-0.8.19.drv'.

@fare
Copy link
Contributor Author

fare commented Mar 19, 2023

I managed to patch the python script so it avoids requiring a tty, and added a few more check dependencies.

It's now at the point where it wants evmone to compile, and as I read the documentation I see it wants hera, too. Sigh. That's a much taller order than fixing the immediate build.

@fare
Copy link
Contributor Author

fare commented Mar 19, 2023

Trying to add an evmone package to nixpkgs, and it's another exercise in yak shaving.

@fare
Copy link
Contributor Author

fare commented Mar 19, 2023

OK, I give up for now. Too many other more important things to do in my life and for my job.

The patch offered is not a regression. Obviously these tests have never worked in Nix. Getting them to work would be a couple few days of work for an engineer.

@fare
Copy link
Contributor Author

fare commented Mar 20, 2023

I punted and made this patch pass the tests by commenting out the invocation of the script that depends on evmone and more, with a TODO comment. This way, solc can fix the known build issues, and still run a lot of tests. Further fixing the tests will require a lot of work.

@fare
Copy link
Contributor Author

fare commented Mar 20, 2023

To upstream my patch to a solidity test: ethereum/solidity#14057

@fare
Copy link
Contributor Author

fare commented Mar 27, 2023

Can we get this merged? The commented out test is not a regression.

@brianmcgee
Copy link
Contributor

Result of nixpkgs-review pr 219240 run on x86_64-linux 1

1 package built:
  • solc

@zimbatm zimbatm merged commit a3cff1f into NixOS:master Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants