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

Reworked setuptools and add github build process #28

Merged

Conversation

remyroy
Copy link
Member

@remyroy remyroy commented Apr 30, 2024

Fixes #11 and #26 .

This removes setuptools as part of main project dependencies, the use of setup.py. It adds a new way to run from the CLI using the standard way to run a module with Python. It updates the related README file sections. It adds ci-build workflow to build release assets using the Github workflow and the Github runners. The ci-build can only be started once it's in the main branch of the repo. In order to test this, you can merge it into your personal main branch and start the workflow manually by going into actions, selecting the ci-build workflow and clicking on Run workflow as detailed in https://docs.github.com/en/actions/using-workflows/manually-running-a-workflow .

There might be more work to be done for a full build and release workflow like a runner for Linux arm64, GPG signing, running tests against the resulting bundled binaries (the tests in test_binary_btec_script.py, test_binary_deposit_script.py, test_btec_script.py and test_deposit_script.py for instance). These efforts can be completed in other PRs.

@remyroy remyroy mentioned this pull request Apr 30, 2024
@remyroy
Copy link
Member Author

remyroy commented Apr 30, 2024

I also removed the use of setup.py as part of the Dockerfile and the Makefile. I've tested the Docker image and the Makefile and they are still working for me after those changes.

Copy link
Collaborator

@valefar-on-discord valefar-on-discord left a comment

Choose a reason for hiding this comment

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

Everything works great. Just one last little question

('../../staking_deposit/key_handling/key_derivation/word_lists/*.txt', './staking_deposit/key_handling/key_derivation/word_lists/'),
('../../staking_deposit/intl', './staking_deposit/intl'),
]
datas += copy_metadata('py_ecc')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Curious why we are doing this as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Both py_ecc and ssz are using importlib for metadata reflection/inspection during module load. It might have been added in a recent version. In any case, the app triggers an unhandled exception if that data is not included in the bundled binary.

@valefar-on-discord valefar-on-discord merged commit 0315d9a into eth-educators:main May 1, 2024
15 checks passed
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.

Remove the use of setuptools installation
2 participants