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

Simplify output from make pyspec #3938

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ generate_tests: $(GENERATOR_TARGETS)

# "make pyspec" to create the pyspec for all phases.
pyspec:
python3 -m venv venv; . venv/bin/activate; python3 setup.py pyspecdev
@python3 -m venv venv; . venv/bin/activate; python3 setup.py pyspecdev

# check the setup tool requirements
preinstallation:
Expand Down
14 changes: 11 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@
)
from pysetup.md_doc_paths import get_md_doc_paths

# Ignore '1.5.0-alpha.*' to '1.5.0a*' messages.
import warnings
warnings.filterwarnings('ignore', message='Normalizing .* to .*')

# Ignore 'running' and 'creating' messages
import logging
class PyspecFilter(logging.Filter):
def filter(self, record):
return not record.getMessage().startswith(('running ', 'creating '))
logging.getLogger().addFilter(PyspecFilter())
Comment on lines +38 to +47
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this stuff tends to be fragile so it could more easily break in the future

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 maybe. Shouldn't be difficult to update if something breaks. Worst case we can just delete it.


# NOTE: have to programmatically include third-party dependencies in `setup.py`.
def installPackage(package: str):
Expand Down Expand Up @@ -394,8 +404,6 @@ def initialize_options(self):
def finalize_options(self):
"""Post-process options."""
if len(self.md_doc_paths) == 0:
print("no paths were specified, using default markdown file paths for pyspec"
Copy link
Member

Choose a reason for hiding this comment

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

does this PR change anything if someone does provide non-default paths?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure. I don't even know how to provide a non-default path. Is this something we do?

Copy link
Member

Choose a reason for hiding this comment

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

not sure if its used but it does seem like the code currently supports it

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, so it doesn't change any functionality with that. I just removed the print call.

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't used the path flags, but Proto did when he designed it: https://github.com/ethereum/consensus-specs/tree/v1.5.0-alpha.6/tests/core/pyspec#advanced

Given that no current use cases require us to only build phase0 specs, I think it’s OK to ignore it for now.

" build (spec fork: %s)" % self.spec_fork)
self.md_doc_paths = get_md_doc_paths(self.spec_fork)
if len(self.md_doc_paths) == 0:
raise Exception('no markdown files specified, and spec fork "%s" is unknown', self.spec_fork)
Expand Down Expand Up @@ -428,6 +436,7 @@ def run(self):
if not self.dry_run:
dir_util.mkpath(self.out_dir)

print(f'Building pyspec: {self.spec_fork}')
for (name, preset_paths, config_path) in self.parsed_build_targets:
spec_str = build_spec(
spec_builders[self.spec_fork].fork,
Expand Down Expand Up @@ -492,7 +501,6 @@ def run_pyspec_cmd(self, spec_fork: str, **opts):
self.run_command('pyspec')

def run(self):
print("running build_py command")
for spec_fork in spec_builders:
self.run_pyspec_cmd(spec_fork=spec_fork)

Expand Down