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

fixes #111 #112

Merged
merged 5 commits into from
Nov 20, 2020
Merged

fixes #111 #112

merged 5 commits into from
Nov 20, 2020

Conversation

Macarse
Copy link
Contributor

@Macarse Macarse commented Nov 18, 2020

What I did

Found the source of the issue:
https://github.com/ethereum/solc-bin/tree/gh-pages/windows-amd64

Starting 0.7.2 they are exes

Related issue: #111

How I did it

mmmm I don't know what you expect me to write here

How to verify it

I am not running windows

Checklist

  • I have confirmed that my PR passes all linting checks
  • I have included test cases
  • I have updated the documentation (README.md)
  • I have added an entry to the changelog

ok, I didn't do any of those. pytest failed

$ pytest tests
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --cov=solcx --cov-branch --cov-report
  inifile: /Users/x/dev/py-solc-x/setup.cfg
  rootdir: /Users/x/dev/py-solc-x

and I am not sure if I will be able to test windows behavior outside of windows.

solcx/install.py Outdated Show resolved Hide resolved
@iamdefinitelyahuman
Copy link
Contributor

Looks like this is failing at least in part due to an unrelated issue. I should be able to look at it soon. Thanks for your help with this!

solcx/install.py Outdated
Comment on lines 613 to 621
if filename.endswith("exe"):
with open(install_path, "wb") as fp:
fp.write(content)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the issue here is that on windows systems, solc is placed in a unique folder for each installation. This is required because zipped versions include a separate DLL file.

The simplest approach would then be to use the same file structure for these new non-zipped executables:

Suggested change
if filename.endswith("exe"):
with open(install_path, "wb") as fp:
fp.write(content)
if filename.suffix == ".exe":
install_path.mkdir()
with open(install_path.joinpath("solc.exe"), "wb") as fp:
fp.write(content)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, do you have a way to test it? I don't have a windows box :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AttributeError: 'str' object has no attribute 'suffix'

Copy link
Contributor

Choose a reason for hiding this comment

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

ooooh... my bad! Path(filename).suffix

Macarse and others added 5 commits November 20, 2020 13:39
Co-authored-by: Just some guy <3859395+fubuloubu@users.noreply.github.com>
Co-authored-by: Ben Hauser <35276322+iamdefinitelyahuman@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants